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

SCAN4NET-171 Read new properties for downloading plugins #2294

Merged
merged 3 commits into from
Feb 4, 2025

Conversation

alex-meseldzija-sonarsource
Copy link
Contributor

@alex-meseldzija-sonarsource alex-meseldzija-sonarsource commented Jan 24, 2025

@hashicorp-vault-sonar-prod hashicorp-vault-sonar-prod bot changed the title SCAN4NET-171 Read new properties for downloading plugins SCAN4NET-171 Read new properties for downloading plugins Jan 24, 2025
@alex-meseldzija-sonarsource alex-meseldzija-sonarsource force-pushed the alex/SCAN4NET-171 branch 2 times, most recently from 0e8ecff to fdc4cef Compare January 24, 2025 12:54
Copy link
Contributor

@pavel-mikula-sonarsource pavel-mikula-sonarsource left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small conceptual nuke to start with

@alex-meseldzija-sonarsource alex-meseldzija-sonarsource force-pushed the alex/SCAN4NET-171 branch 2 times, most recently from 51fadff to ad8f09f Compare January 31, 2025 10:40
@alex-meseldzija-sonarsource alex-meseldzija-sonarsource changed the base branch from master to alex/refactor-fetch-analyzer-plugin January 31, 2025 10:51
@alex-meseldzija-sonarsource alex-meseldzija-sonarsource force-pushed the alex/refactor-fetch-analyzer-plugin branch 2 times, most recently from 5fb0eda to 1e11364 Compare January 31, 2025 13:26
@alex-meseldzija-sonarsource alex-meseldzija-sonarsource force-pushed the alex/refactor-fetch-analyzer-plugin branch from 1e11364 to f4efaca Compare January 31, 2025 14:45
@alex-meseldzija-sonarsource alex-meseldzija-sonarsource force-pushed the alex/SCAN4NET-171 branch 2 times, most recently from 98affdf to f089335 Compare January 31, 2025 14:57
Base automatically changed from alex/refactor-fetch-analyzer-plugin to master January 31, 2025 15:20
Copy link
Contributor

@pavel-mikula-sonarsource pavel-mikula-sonarsource left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some polishing

Copy link
Contributor

@pavel-mikula-sonarsource pavel-mikula-sonarsource left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final cleanup

{"sonaranalyzer.security.cs.staticResourceName", "OLDSecurityCSharpFrontend" },
});
var context = new Context(TestContext, sonarProperties, [[@"c:\assembly1.dll"], [@"d:\foo\assembly2.dll"]], null, language);
var securityProperties = language == RoslynAnalyzerProvider.CSharpLanguage ?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

securityProperties and expectedSonarLintXml are the same for RoslynConfig_ValidProfile_WithLegacy and RoslynConfig_ValidProfile_WithoutLegacy.

Extract it. And use the same scaffolding (add security) also in RoslynConfig_ValidProfile_LegacyOnly. Then it will be used 3x.

Comment on lines +187 to +195
var expectedPlugins = new List<Plugin>()
{
new() { Key = "wintellect", Version = "1.13.0", StaticResourceName = "wintellect.zip" },
new() { Key = language, Version = "1.42.0", StaticResourceName = "SonarAnalyzer.zip" },
};
if (language == RoslynAnalyzerProvider.CSharpLanguage)
{
expectedPlugins.Add(new() { Key = "securitycsharpfrontend", Version = "1.42.0", StaticResourceName = "SecurityAnalyzer.zip" });
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calculation of expectedPlugins should be also extracted, as it will be used 3x

Copy link
Contributor

@pavel-mikula-sonarsource pavel-mikula-sonarsource left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I'll merge it as-is and will address the comments in another PR

@pavel-mikula-sonarsource pavel-mikula-sonarsource merged commit c4cb8c9 into master Feb 4, 2025
19 checks passed
@pavel-mikula-sonarsource pavel-mikula-sonarsource deleted the alex/SCAN4NET-171 branch February 4, 2025 08:04
Comment on lines +142 to +153
<Setting>
<Key>sonar.{language}.analyzer.dotnet.pluginKey</Key>
<Value>{language}</Value>
</Setting>
<Setting>
<Key>sonar.{language}.analyzer.dotnet.pluginVersion</Key>
<Value>1.42.0</Value>
</Setting>
<Setting>
<Key>sonar.{language}.analyzer.dotnet.staticResourceName</Key>
<Value>SonarAnalyzer.zip</Value>
</Setting>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oversight from previous review: We shouldn't generate these to SonarLint.xml file, as sonaranalyzer-{language} where previously also not generated.

We use them to talk to scanner. We should not propagate them back to the analyzers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants