[WIP] Merge RoslynAnalyzers into Roslyn #77178
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…6c-802f-2a02428d069e [main] Update dependencies from dotnet/arcade
CA2249: Also trigger on `String.IndexOf() != -1`
Fixes dotnet#6616 We now ensure that setting an abstract value for an indexed access entity with any non-constant index clears out the existing value for entities that can overlap the indexed location
CA1853: Fix incorrect code fixes
Improve handling of indexed array access in flow analysis
We used to only track reference types and strings in PointsToAnalysis, which in turns leads to `ConfiguredAsyncDisposable` struct type returned from `TaskAsyncEnumerableExtensions.ConfigureAwait(this IAsyncDisposable, bool)` not being tracked, and hence the `await using` statement doesn't mark the original disposable object as being disposed.
Track all disposable types in PointsToAnalysis
… 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
Update to Microsoft.CodeAnalysis.Testing 1.1.2-beta1.24314.1
…-override CA2213: Handle overriden `DisposeCoreAsync` / `DisposeAsyncCore`
…otnet#7387) * Temporarily disable pragma SYSLIB0014 from CSharp CodeFix Verifiers. * Also suppress SYSLIB0014 in DataSetDataTableInWebSerializableObjectGraphTests.cs
…templates) (dotnet#7286) * Stabilize package versions (dotnet#7002) * Revert "Stabilize package versions (dotnet#7002)" (dotnet#7003) This reverts commit e640355940de88119119feb5da0c51086c711759. * Adds validation against invalid bracket pairs * Fixes dotnet#7285 * Fix extra break flagged by CI * Remove one branch to be fully covered * Adds secondary occurrence of CA2017 around mismatched braces * Adds new messages to base resource file * Ran resource generation * Fix tests, use `WorkItem` attribute * documentation updates for CA2017 * Revert "documentation updates for CA2017" This reverts commit 4c41c4e6a6f0f3e8cee2edf8a3848df783fa8cc1. * Trying something else with CA2017 regarding localization * Needed to have a title and description that match *both* potential reasons for this warning * The individual messages for the differing reasons is still separate, but the MD/sarif description seemed to be "last wins" when it comes to a title/description * The `msbuild /t:pack` command kept failing for me, so upped the global.json to target a non preview .net8 SDK, but am not checking that change in * Finish rebase * Take into account escaped braces * Use CA2023 instead of an overloaded CA2017 * Revert "Ran resource generation" This reverts commit 26f1e68e8a4809f6e044c8cd9e5606bb697e2fb4. * Revert "Adds new messages to base resource file" This reverts commit 386fe964e153e62d20d1ac2f4c701b78332915f7. * Adds new CA2023 information to resources * Will likely need a follow up commit to "undo" the "new" tags around `CA2017`, as these should now match from main * Reverts "new" translation indicators back to their previously "translated" versions from CA2017 * CA2017 was originally repurposed, which caused undesired impacts to already released versions of the analyzer. Instead these changes are introduced as a new CA2023 dotnet/roslyn-analyzers#7286 (comment) * Missed a few files from last commit * More covering tests, comment tweaking * Remove additional space between sentences, `error` -> `warning` * fix(LoggerMessageDefineAnalyzer): BuildError -> BuildWarning * Optimized impl from @tarekgh * linting and another missed `error` -> `warning` * Additional covering tests --------- Co-authored-by: Viktor Hofer <[email protected]>
* CA1854: Use unused variable name for out parameter This commit also adds a new convenience extension method for other fixers to use. * Run msbuild pack
* Add an analyzer for Debug.Assert As @jaredpar found in dotnet#75163, interpolated strings in `Debug.Assert` can consume a surprising amount of memory. On modern .NET, this is fine; `Debug.Assert` has an interpolated string handler that will avoid the allocations if the assert isn't triggered. However, on our framework tests, this can be very bad and OOM our tests. So, this analyzer looks for cases where interpolated strings are passed to `Debug.Assert`, and recommends moving over to `RoslynDebug.Assert` instead, which is an interpolated string handler on all platforms. Note that I only did C# support, as there's no equivalent handler API for VB. * Make sure RoslynDebug is available in one test. * Trailing whitespace * Run pack. * Remove VB * Pack again * Use batch fixall provider * Avoid passing empty string as command line argument See microsoft/dotnet#1062 * Don't warn for constant strings. * Apply suggestions from code review Co-authored-by: Jared Parsons <[email protected]> --------- Co-authored-by: Sam Harwell <[email protected]> Co-authored-by: Jared Parsons <[email protected]>
We're officially marking `ISourceGenerator` as deprecated, but we can't actually deprecate the interface, because it's used in several APIs like AnalyzerFileReference. So instead, we just add an analyzer to indicate that the interface should not be implemented. I've made this more general so we can do this with more interfaces in the future if necessary, based the implementation on InternalImplementationOnlyAnalyzer.
…otnet#7390) * Add CA2024: Do not use 'StreamReader.EndOfStream' in async methods This analyzer detects when 'StreamReader.EndOfStream' is used in an async method, which can prevent I/O from being done asynchronously. * Use VerifyAnalyzerAsync instead of VerifyCodeFixAsync * Explicitly specify diagnostic ID in test markup * Check symbols first before checking async context Using the Stream type, or more specifically, the EndOfStream property of the Stream type, should be much rarer than async methods in general, so we check the symbols first. * Remove IsAsyncContext * Remove unnecessary containing type check
…rcegenerator/no-warns [ResxSourceGenerator] Add a `NoWarn` attribute
…#7501) * Error when users use a `file` type for things we need to load Inspired by dotnet#76355. `file` types can cause errors on .NET Framework hosts, and we should just prevent these from being used. The purpose of `file` types is to not be visible outside the current file, and the purpose of analyzers is to be loaded by some other plugin system. These goals are mutually exclusive. * Seal type * Feedback.
…dotnet#7542) * Ensure that PlatformCompatibilityAnalyzer better handles various TFMs * Fix a typo * Fixing up the PlatformCompatibilityAnalyzer tests to pass down the new expected properties
* Add CA1873: Avoid potentially expensive logging This analyzer detects calls to 'ILogger.Log', extension methods in 'Microsoft.Extensions.Logging.LoggerExtensions' and methods decorated with '[LoggerMessage]'. It then checks if they evaluate expensive arguments without checking if logging is enabled with 'ILogger.IsEnabled'. * Use loop for IsGuardedByIsEnabled instead of recursion * Rename TryGetLogLevel to IsLogInvocation * Rework how expensive argument evaluation is detected Previously, some specific operations were treated as non-expensive and the rest were. This is not really future-proof as the language evolves, and is generally more prone to false positives. So this has now been changed to the opposite: Treat certain operations as expensive, and the rest are not expensive by default and do not trigger a diagnostic. * Fix same instance detection when using conditional access * Run msbuild pack * Reorder tests * Fix constant interpolation string detection * Cleanup and add missing tests * Add StringSyntax attribute to highlight test source * Allow nameof, consts and literals in interpolated strings * Use SymbolEqualityComparer to check same instance symbol * Check all parent blocks for IsEnabled guard
dotnet-issue-labeler
bot
added
Area-Infrastructure
untriaged
dotnet-policy-service
bot
added
the
Needs API Review
|
This PR modifies public API files. Please follow the instructions at https://github.com/dotnet/roslyn/blob/main/docs/contributing/API%20Review%20Process.md for ensuring all public APIs are reviewed before merging. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is not ready for review.