Skip to content

[release/8.0.1xx] Handle reference to non-source symbols in UseConcreteTypeAnalyzer #6960

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 22, 2023

Conversation

github-actions[bot]
Copy link

@github-actions github-actions bot commented Sep 21, 2023

Backport of #6959 to release/8.0.1xx
Issue: IndexOutOfRangeException in UseConcreteTypeAnalyzer (CA1859)

/cc @mavasani

Customer Impact

CA1859: Use concrete types when possible for improved performance analyzer throws IndexOutOfRangeException at runtime when the analyzing symbol is not defined within the source.

This issue is reported by internal and external users, more users could be affected after the .NET 8 release. The analyzer newly added in .NET 8, enabled by default at Info level.

Testing

A unit test that reproes the bug added.

Risk

Very low - the fix would not introduce a new warning; the fix is straightforward and unit tested

@github-actions github-actions bot requested a review from a team as a code owner September 21, 2023 17:29
@codecov
Copy link

codecov bot commented Sep 21, 2023

Codecov Report

Merging #6960 (ca17759) into release/8.0.1xx (2c9a20b) will decrease coverage by 0.01%.
The diff coverage is 80.00%.

@@                 Coverage Diff                 @@
##           release/8.0.1xx    #6960      +/-   ##
===================================================
- Coverage            96.39%   96.39%   -0.01%     
===================================================
  Files                 1402     1402              
  Lines               332092   332095       +3     
  Branches             11057    11059       +2     
===================================================
- Hits                320131   320120      -11     
- Misses                9194     9207      +13     
- Partials              2767     2768       +1     

Copy link
Member

@jeffhandley jeffhandley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I support this based on it being a broken scenario in a new feature in .NET 8, with the issue being reported by a partner team using the preview build. Over to @artl93.

@artl93
Copy link
Member

artl93 commented Sep 21, 2023

@jeffhandley - is there a general pattern in this scenario here we should be considering validating more broadly? (E.g. Symbols not defined in the source).

Copy link
Member

@artl93 artl93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

M2 approved.

@carlossanlop
Copy link

Approved by Tactics via email. Ready to merge.

@carlossanlop carlossanlop merged commit 4a7701f into release/8.0.1xx Sep 22, 2023
@carlossanlop carlossanlop deleted the backport/pr-6959-to-release/8.0.1xx branch September 22, 2023 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants