Skip to content
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

Merge RoslynAnalyzers into Roslyn #77617

Open
wants to merge 5,660 commits into
base: main
Choose a base branch
from

Conversation

JoeRobich
Copy link
Member

@JoeRobich JoeRobich commented Mar 15, 2025

This PR brings over the roslyn owned packages from the roslyn-analyzers repo along with their history. The goal of this PR is to get the analyzers moved over in a state that we can live with so that they can be removed from roslyn-analyzers. We can then do further reorganization and integration as follow-ups. My changes begin at commit 24fbf05.

NetAnalyzers will remain in roslyn-analyzers.

Moved packages:

  • Microsoft.CodeAnalysis.Analyzers
  • Microsoft.CodeAnalysis.AnalyzerUtilities
  • Microsoft.CodeAnalysis.BannedApiAnalyzers
  • Microsoft.CodeAnalysis.Metrics
  • Microsoft.CodeAnalysis.PerformanceSensitiveAnalyzers
  • Microsoft.CodeAnalysis.PublicApiAnalyzers
  • Microsoft.CodeAnalysis.ResxSourceGenerator
  • Microsoft.CodeAnalysis.RulesetToEditorconfigConverter
  • Roslyn.Diagnostic.Analyzers

The package versioning is maintained and separate from the current Roslyn package versioning. These packages use a variety of Microsoft.CodeAnalysis package versions and I have maintained that despite our use of transitive version pinning. I have also maintained Roslyn's use of these packages from our NuGet feeds.

I have organized the code under a src/RoslynAnalyzers folder.
image
The projects are organized in our solution file beneath an Analyzers folder.
image

sharwell and others added 30 commits July 21, 2023 09:23
… point to multiple different objects

Fixes dotnet#6520
Now we store an additional optional EntityForInstanceLocation for analysis entities that can point to more than one possible locations across all control flow paths. This is required to distinguis from other entities that can point to the same set of potential locations, but the actual runtime value of the pointed to location may not be the same for both these entities.
Fix race condition in CA1001 analyzer
Handle default expression value properly in Value content analysis
Make flow analysis more conservative in presence of entities that can point to multiple different objects
…1508

Bail out from conversion inference for user-defined conversions
Fixes dotnet#6048
`goto case DiscardPattern` had the required logic for setting the predicated value the pattern value in true branch of is expression, but it was also setting `predicateValueKind`, which is in turn used to identify redundant duplicate conditional checks. Latter is fixed by this change.
Fix CA1508 false positive for pattern expressions
dotnet#6799)

* Implement UseStringMethodCharOverloadWithSingleCharacters analyzer with fixer

* Use StringComparison's InvariantCulture

* Use NotNullWhen instead

* Verify diagnostic message

* Pack

* Rename

* Add more tests

* Rename

* Refactor a bit

* Use ImmutableArray instead of set

* Remove unneeded check

* Add link comment for IsASCII

* Move resolving StringComparison related symbols to compilation start

* Use IsPrintableAscii

* Detect CultureInfo argument too and set the relevant comparison

* Pass symbols used in AnalyzeOperation instead of storing in fields

* Rename

* Preserve certain args (IndexOf/LastIndexOf)

* Remove unneeded

* Update description

* Pack

* Use else if

* Use WellKnownTypeNames

* Update message to use just one arg for the method
…0-ae1d-34557bfc0847

Localized file check-in by OneLocBuild Task: Build definition ID 830: Build ID 2230168
… and local function delegates

Fixes dotnet#5684

We were already performing escape analysis since dotnet#4113. However, we were not propertly resetting all the analysis data on encountering such escapes.
In future, we can consider making the analysis more aggressive for such escapes by not resetting the entire analysis data on escapes, but instead just resetting the analysis data pertaining to capture variables and any points to locations reachable from these variables.
…tive

Add conservative reset of analysis data in presence of escaped lambda and local function delegates
…ferencePackages

Move to latest source build reference packages
* Add analyzer and fixer for CA1865

This analyzer detects when either `Set.Add` or `Set.Remove` is guarded
by `Set.Contains`. A fix is only suggested when the conditional body
has a single statement that is either one of the former two methods.

* Add CA1865 to Microsoft.CodeAnalysis.NetAnalyzers.md

* Add CA1865 to RulesMissingDocumentation.md

* Update DoNotGuardSetAddOrRemoveByContainsDescription

* Add ExportCodeFixProviderAttribute

* Remove call to First with call to indexer

* Change equivalenceKey of CodeAction

* Check for interface implementation instead of signature match

* Remove call to First with call to indexer

* Add additional tests

* Fix ifs with additional statements

* Fix single line ifs in VB

* Add fixer case for single line ifs in VB

* CA1865: Add tests for other set types

* CA1865: Check descendants instead of switch

* CA1865: Do not fire when arguments are different

* CA1865: Change fixer title

* CA1865: Update resources

* WIP: Try to handle IImmutableSet Add and Remove

* Apply suggestions from code review

* CA1865: Use original definitions to filter methods

* Check parameter length before using them

* Only check child operations instead of descendants

This ensures that nested Add or Remove calls are not flagged.

* Fix diagnostic message

* Also flag when interface types are used

* Use helper class for required symbols

* Only compare first argument value

This is sufficient as each method involved only has one argument.

* Make symbol display format static

* Update DoNotGuardSetAddOrRemoveByContainsTitle

* Change condition to pattern matching

* Add tests for ternary operators

* Use raw strings and inline class and usings

* Support Add or Remove in else block

* Support ternary operator (diagnostic only)

* Move from CA1865 to CA1868

---------

Co-authored-by: Buyaa Namnan <[email protected]>
Co-authored-by: Sam Harwell <[email protected]>
# Conflicts:
#	src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif
#	src/Utilities/Compiler/WellKnownTypeNames.cs
Avoid reporting CA1823 for inline arrays
Unify HasAttribute and HasAnyAttribute
@JoeRobich JoeRobich requested review from a team as code owners March 15, 2025 00:11
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Infrastructure untriaged Issues and PRs which have not yet been triaged by a lead labels Mar 15, 2025
@dotnet-policy-service dotnet-policy-service bot added the Needs API Review Needs to be reviewed by the API review council label Mar 15, 2025
@JoeRobich
Copy link
Member Author

@ViktorHofer I copied your changes from dotnet/roslyn-analyzers#7560, but am still seeing .NETStandard 1.x errors. Do I need to add Microsoft.CodeAnalysis to the Version.Details?

@ViktorHofer
Copy link
Member

ViktorHofer commented Mar 17, 2025

@JoeRobich
Copy link
Member Author

@dotnet/source-build for a review

@MartyIX
Copy link
Contributor

MartyIX commented Mar 17, 2025

This PR brings over the roslyn owned packages from the roslyn-analyzers repo along with their history.

Just out of curiosity: How that "history" part is done?

@JoeRobich
Copy link
Member Author

JoeRobich commented Mar 17, 2025

Just out of curiosity: How that "history" part is done?

In my local clone of roslyn-analyzers, I moved the code that we are bringing over beneath the /src/RoslynAnalyzers folder and made a commit so that history could be followed. I then ran git filter-branch --prune-empty ... for that folder. This modifies my branch so that only those files exist and only the commits that changed those files are kept. This does rewrite the commits, so SHAs aren't preserved. I then added my local roslyn-analyzers repo as a remote in my cloned roslyn repository. I was able to fetch my modified branch and then merge it in with roslyn main using git merge --allow-unrelated-histories.

  • Edited to include that I did make a commit after moving the analyzer source to a new folder prior to filtering the history.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Infrastructure Needs API Review Needs to be reviewed by the API review council untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.