Decompose SymbolHelper into dedicated extension classes #9229
Conversation
analyzers/tests/SonarAnalyzer.Test/Extensions/ITypeSymbolExtensionsTest.cs
Outdated
Show resolved
Hide resolved
| #if DEBUG | ||
| new Action(() => ISymbolExtensionsCommon.GetClassification(fakeSymbol.Object)).Should().Throw<NotSupportedException>(); | ||
| #else | ||
| ISymbolExtensionsCommon.GetClassification(fakeSymbol.Object).Should().Be("symbol"); | ||
| #endif |
There was a problem hiding this comment.
What is the point of the #if DEBUG .. #else .. #endif?
Do we run UTs in both Debug and Release?
There was a problem hiding this comment.
The implementation behaves differently under debug and release builds
There was a problem hiding this comment.
I mean, if we run UTs in both configurations (Debug & Release), it is fine. Otherwise, I don't see the point to have these.
Do we run UTs in both configurations in the CI?
There was a problem hiding this comment.
We run release in CI and debug locally
analyzers/tests/SonarAnalyzer.Test/Extensions/ITypeSymbolExtensionsTest.cs
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.Test/Extensions/ISymbolExtensionsTest.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.Test/Extensions/IPropertySymbolExtensionsTest.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.VisualBasic/Extensions/SyntaxNodeExtensions.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.Test/Trackers/BuilderPatternConditionTest.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Shared/Extensions/SyntaxNodeExtensions.cs
Outdated
Show resolved
Hide resolved
|
|
| using ExtensionsCommon = common::SonarAnalyzer.Extensions.SyntaxNodeExtensions; | ||
| using ExtensionsCS = csharp::SonarAnalyzer.Extensions.SyntaxNodeExtensionsCSharp; | ||
| using ExtensionsVB = vbnet::SonarAnalyzer.Extensions.SyntaxNodeExtensionsCSharp; | ||
| using ExtensionsShared = csharp::SonarAnalyzer.Extensions.SyntaxNodeExtensionsShared; |
There was a problem hiding this comment.
For my understanding, shared projects behave as they were part of the assembly they are imported in?
That's why we have the chasrp:: namespace prefix here?
There was a problem hiding this comment.
Exactly. There exists vbnet:SonarAnalyzer.Extensions.SyntaxNodeExtensionsShared AND csharp:SonarAnalyzer.Extensions.SyntaxNodeExtensionsShared.
We're testing just one of them.
| IEnumerable<(SyntaxNode node, string name)> GetCSharpNodes() => | ||
| snippet.GetNodes<CSharpSyntax.InvocationExpressionSyntax>() | ||
| .Select(n => ((SyntaxNode)n, CSharpSyntaxNodeExtensions.GetIdentifier(n.Expression)?.ValueText)); | ||
| IEnumerable<(SyntaxNode Node, string Name)> GetCSharpNodes() => |
There was a problem hiding this comment.
This is not raising T0003 (ValueTuple)?
There was a problem hiding this comment.
No, because it's a test code. See
Review per commit should be easier, as methods were moved to many different places, and few file scoped namespaces were migrated.
IsAnyAttributeIsOverridingChainandGetAllNamedTypeswere misplaced in their original commits. There are commits later that fixes these