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
SonarLintXmlReader refactoring #6924
Conversation
262b3b7
to
e841ab6
Compare
49b29b9
to
bc2a5d2
Compare
285749b
to
09536d2
Compare
1796226
to
9289fca
Compare
analyzers/src/SonarAnalyzer.Common/Helpers/SonarLintXmlReader.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just one nitpick comment
analyzers/src/SonarAnalyzer.Common/AnalysisContext/SonarAnalysisContextBase.cs
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Helpers/SonarLintXmlReader.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kaboom
analyzers/src/SonarAnalyzer.Common/Helpers/SonarLintXmlReader.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Helpers/SonarLintXmlReader.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Helpers/SonarLintXmlReader.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Helpers/SonarLintXmlReader.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Helpers/SonarLintXmlReader.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Helpers/SonarLintXmlReader.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
analyzers/src/SonarAnalyzer.Common/Helpers/SonarLintXmlReader.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Pavel Mikula <57188685+pavel-mikula-sonarsource@users.noreply.github.com>
c555e5a
to
00c54dc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There are some minor UT improvements to do
@@ -82,6 +82,22 @@ public void SonarLintXmlReader_PropertiesCSharpTrueVBNetFalse_ExpectedValues() | |||
sut.AnalyzeGeneratedCode(LanguageNames.VisualBasic).Should().BeFalse(); | |||
} | |||
|
|||
[TestMethod] | |||
public void SonarLintXmlReader_DuplicatedProperties_DoesNotFail() | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Throwing is asserted via lambdas.
{ | |
public void SonarLintXmlReader_DuplicatedProperties_DoesNotFail() => | |
((Action)(() => CreateSonarLintXmlReader("ResourceTests\\SonarLintXml\\Duplicated_Properties\\SonarLint.xml"))).Should().NotThrow(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I strongly prefer @"asdf\asdf"
over double \\
- this applies everywhere. The \\
is cumbersome to work with, copy paste and use in general.
Kudos, SonarCloud Quality Gate passed! |
Kudos, SonarCloud Quality Gate passed! |
Addresses comments from #6870