Skip to content

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

mavasani and others added 30 commits July 25, 2023 19:54
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 <buyankhishig.namnan@microsoft.com>
Co-authored-by: Sam Harwell <sam@tunnelvisionlabs.com>
# Conflicts:
#	src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif
#	src/Utilities/Compiler/WellKnownTypeNames.cs
Avoid reporting CA1823 for inline arrays
Unify HasAttribute and HasAnyAttribute
This change minimizes the number of cases in code that need to account
for switching between public and internal accessibility based on
per-file options.
Fixes dotnet#5199
Fixes dotnet#5160

We already had implementation and option based support to perform exception path analysis to handle all operations that can potentially throw an exception, and hence need flow analysis data at the point of operation to be merged with analysis data at start of reachable catch blocks from that operation. However, we were only performing this analysis post pass when flow analysis is looking at exceptions data. Now we do it for all analysis as this is required for more accurate input analysis data at start of catch and finally blocks.
NOTE: Fixing this issue led to an assert firing in CodeAnalysis dataflow operation visitor for an existing unit test, which has also been fixed in this PR
@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.

@JoeRobich JoeRobich merged commit cdcc3c4 into dotnet:main Mar 31, 2025
28 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Mar 31, 2025
@jjonescz jjonescz modified the milestones: Next, 17.14 P3 Apr 1, 2025
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.