SonarSource/sonar-scanner-msbuild

Multi-Language Support: Test plan

gregory-paidis-sonarsource opened this issue · 19 comments

gregory-paidis-sonarsource commented

Background Tasks

  • Allow non-C# files to be analyzed on peach on specific projects that have been added for this sprint. (Done)
  • Bump JS/TS plugin version on orchestrator ITs to test rules that are test-scoped.
  • Bump the scanner version on peachee to test multi-file analysis.
  • Run the build script containing npm install in one of the multi-file projects.

Manual Scenarios

  • Test UI Warning on local SQ/SC. #2084 (comment)
  • Test detection for .spec.js, .spec.ts, .test.js, test.ts, etc. This is not possible as an IT because we pin the version of JS/TS and there are no test-scoped rules on the current version. See here #2084 (comment)
  • Manually overridden sonar.languageXXX.file.suffixes from the server. #2084 (comment)
  • Manually overridden sonar.languageXXX.file.suffixes from the command line.
  • Make a project, analyze it with the old scanner and then analyze it with the latest master scanner and see if the file move works correctly with a different root. #2084 (comment)
  • Test .esproj with Include/Exclude contents. #2084 (comment)
  • obj, bin, publish directories under .gitignore. Make a new project with npm+dotnet, run a first build on both and then run begin-build-end, see what happens. #2084 (comment)
  • Disable MultiFileAnalysis via the flag. We should not see files that are not part of the solution analyzed. #2084 (comment)
  • User-specified sonar.tests. We should see sources set to everything we found and tests set to whatever the user specified. #2084 (comment)
  • node_modules (e.g. npx create-react-app my-app --template typescript) CRA is deprecated but for our use case it is ideal because it has tons of dependencies. #2084 (comment)
  • Talk with JS/TS squad to see if they have any project ideas to test manually https://sonarsource.slack.com/archives/CFY6WGR63/p1722520460640169 - tested a few of them locally
  • Test the happy flow with the previous version of SQ #2084 (comment)
  • Test with the scm disabled #2084 (comment)
  • Check Andrei's email about other projects to add on peach - we can test them manually for now. https://github.com/SonarSource/peachee-dotnet/pull/140
  • Duplication problems:
    • ts, jsx. We may see duplicates in the analysis (e.g. the ts and the transpiled js file). We can use angular/react SPA template.
    • sass and less same problem but for css. We can use angular/react SPA template. #2138
    • static HTML files. This is for an MVC app with some npm integration. MVC outputs the code to the publish directory and npm doesn't transpilation. As part of the MVC we also have static assets, like e.g. HTML files. The HTML files might be seen twice.
    • Hidden directories starting with a . like .git .vs .rider ad so on. These may include HTML or js files for some reason. #2084 (comment)
martin-strecker-sonarsource commented

Thinks we discussed that need to be tested:

  • node_modules (e.g. npx create-react-app my-app --template typescript) CRA is deprecated but for our use case it is ideal because it has tons of dependencies.
  • obj, bin, and publish directories. We need to come up with a way to produce files there. A possible solution could be using one of the integrated SPA templates in VS. These create an backend (asp.net core API) and an SPA. The SPA get's likely bundled as minified js in the bin directory and maybe some intermediate files are dropped in obj as well. Nice read about the setup and csproj/package.json interdependencies is here. We should try some of the old SPA templates from older .Net SDKs because each did it differently than the one before. Usefull commands: dotnet new list --tag SPA dotnet new global.json --sdk-version 3.1.0 --roll-forward latestFeature --force react only comes with js but the angular template comes with ts. react use CRA (and webpack) angular I don't know.
  • Duplication problems:
    • ts, jsx, and other transpiled files. We may see duplicates in the analysis (e.g. the ts and the transpiled js file),
    • sass and less same problem but for css. We may see duplicates
    • static HTML files. This is for an MVC app with some npm integration. MVC outputs the code to the publish directory and npm doesn't transpilation. As part of the MVC we also have static assets, like e.g. HTML files. The HTML files might be seen twice.
  • Hidden directories starting with a . like .git .vs .rider and so on. These may include HTML or js files for some reason.
gregory-paidis-sonarsource commented

Scanner has been baked in peachee, see here

gregory-paidis-sonarsource commented

kavita dump of sonar-project.properties.
SCM support was disabled, and it generated a lot of .xml, .json. and noise.

Snippet:

sonar.sources=\
"C:\\sonar-ci\\Project\\.sonarqube\\out\\2\\ProjectInfo.xml",\
"C:\\sonar-ci\\Project\\.vscode\\launch.json",\
"C:\\sonar-ci\\Project\\.vscode\\settings.json",\
"C:\\sonar-ci\\Project\\API\\bin\\Debug\\API.xml",\
"C:\\sonar-ci\\Project\\API\\bin\\Debug\\net8.0\\API.deps.json",\
"C:\\sonar-ci\\Project\\API\\bin\\Debug\\net8.0\\API.runtimeconfig.json",\
"C:\\sonar-ci\\Project\\API\\bin\\Debug\\net8.0\\API.xml",\
"C:\\sonar-ci\\Project\\API\\bin\\Debug\\net8.0\\config\\appsettings.Development.json",\
"C:\\sonar-ci\\Project\\API\\bin\\Debug\\net8.0\\config\\appsettings.json",\
"C:\\sonar-ci\\Project\\API\\bin\\Debug\\net8.0\\EmailTemplates\\base.html",\
"C:\\sonar-ci\\Project\\API\\bin\\Debug\\net8.0\\EmailTemplates\\EmailChange.html",\
"C:\\sonar-ci\\Project\\API\\bin\\Debug\\net8.0\\EmailTemplates\\EmailConfirm.html",\

sonar-project.zip

gregory-paidis-sonarsource commented

If a user before bumping the scanner had set scm.disabled to true, they will get a lot of noise from this update.
Martin:
An idea is to do A-B Testing and also only support tsql/plsql at the first iteration.

gregory-paidis-sonarsource commented

From peach testing:
We need to disable .sonarqube and .sonar directories from being analyzed.

costin-zaharia-sonarsource commented

Please keep in mind that scm is disabled on peachee. See here: https://github.com/SonarSource/peachee-dotnet/blob/dotnet/scanner-begin.ps1#L12

We might want to not set that parameter for the multi-file analysis projects.

martin-strecker-sonarsource commented 
dotnet new react -n CRA
cd CRA
git init
git commit -m "Initial commit"
dotnet build <-- This also runs "npm install" populating the node_modules folder
dotnet E:\dl\SonarScanner.MSBuild.dll begin /k:CRA /d:sonar.host.url=http://localhost:9000 /d:sonar.login=squ_***
dotnet build --no-incremental
dotnet E:\dl\SonarScanner.MSBuild.dll end /d:sonar.login=squ_***

Results in

INFO: Preprocessing files...
INFO:
INFO: 5 languages detected in 37 preprocessed files
INFO: 29982 files ignored because of scm ignore settings
INFO: Loading plugins for detected languages
INFO: Load/download plugins
INFO: Load/download plugins (done) | time=24ms
INFO: Executing phase 2 project builders
INFO: Executing phase 2 project builders (done) | time=22ms
INFO: Load project repositories
INFO: Load project repositories (done) | time=5ms
INFO: Indexing files...
INFO: Project configuration:
INFO: Indexing files of module 'CRA'
INFO:   Base dir: C:\Projects\Sprints\MultiFile\CRA
INFO:   Source paths: Controllers/WeatherForecastController.cs, Pages/Error.cshtml....
INFO:   Test paths: ClientApp/src/App.test.js, ClientApp/node_modules/postcss-clamp...
INFO: Indexing files of module 'CRA'
INFO:   Base dir: C:\Projects\Sprints\MultiFile\CRA
INFO:   Test paths: ClientApp/src/App.test.js, ClientApp/node_modules/postcss-clamp...
INFO: ------------------------------------------------------------------------
INFO: EXECUTION FAILURE
INFO: ------------------------------------------------------------------------
INFO: Total time: 22.033s
INFO: Final Memory: 16M/96M
ERROR: Error during SonarScanner execution
INFO: ------------------------------------------------------------------------
ERROR: File ClientApp/src/App.test.js can't be indexed twice. Please check that inclusion/exclusion patterns produce disjoint sets for main and test files
ERROR:
The SonarScanner did not complete successfully
16:08:41.095  Post-processing failed. Exit code: 1

The sonar-project.properties looks like so

sonar-project.zip

Excerpt:

sonar.projectKey=CRA
sonar.working.directory=C:\\Projects\\Sprints\\MultiFile\\CRA\\.sonarqube\\out\\.sonar
sonar.projectBaseDir=C:\\Projects\\Sprints\\MultiFile\\CRA
sonar.pullrequest.cache.basepath=C:\\Projects\\Sprints\\MultiFile\\CRA
sonar.tests=\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\src\\App.test.js",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\node_modules\\postcss-clamp\\index.test.js",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\node_modules\\camel-case\\dist\\index.spec.js",\
...
17407E65-4BE7-47B8-895A-C27074BD4BEE.sonar.projectKey=CRA:17407E65-4BE7-47B8-895A-C27074BD4BEE
17407E65-4BE7-47B8-895A-C27074BD4BEE.sonar.projectName=CRA
17407E65-4BE7-47B8-895A-C27074BD4BEE.sonar.projectBaseDir=C:\\Projects\\Sprints\\MultiFile\\CRA
17407E65-4BE7-47B8-895A-C27074BD4BEE.sonar.sourceEncoding=utf-8
17407E65-4BE7-47B8-895A-C27074BD4BEE.sonar.sources=\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\Controllers\\WeatherForecastController.cs",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\Pages\\Error.cshtml.cs",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\Program.cs",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\WeatherForecast.cs",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\obj\\Debug\\net7.0\\CRA.GlobalUsings.g.cs",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\obj\\Debug\\net7.0\\.NETCoreApp,Version=v7.0.AssemblyAttributes.cs",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\obj\\Debug\\net7.0\\CRA.AssemblyInfo.cs",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\obj\\Debug\\net7.0\\CRA.RazorAssemblyInfo.cs",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\appsettings.Development.json",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\appsettings.json",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\Pages\\Error.cshtml",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\Pages\\_ViewImports.cshtml",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\.gitignore",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\Properties\\launchSettings.json",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\.env",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\.env.development",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\.gitignore",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\aspnetcore-https.js",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\aspnetcore-react.js",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\package-lock.json",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\package.json",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\public\\favicon.ico",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\public\\index.html",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\public\\manifest.json",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\README.md",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\src\\App.js",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\src\\AppRoutes.js",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\src\\components\\Counter.js",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\src\\components\\FetchData.js",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\src\\components\\Home.js",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\src\\components\\Layout.js",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\src\\components\\NavMenu.css",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\src\\components\\NavMenu.js",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\src\\custom.css",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\src\\index.js",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\src\\reportWebVitals.js",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\src\\service-worker.js",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\src\\serviceWorkerRegistration.js",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\src\\setupProxy.js",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\obj\\CRA.csproj.nuget.dgspec.json",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\obj\\project.assets.json",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\.sonarqube\\conf\\SonarQubeAnalysisConfig.xml",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\node_modules\\.package-lock.json",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\.sonarqube\\conf\\0\\SonarProjectConfig.xml",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\.sonarqube\\conf\\cs\\SonarLint.xml",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\.sonarqube\\conf\\vbnet\\SonarLint.xml",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\.sonarqube\\out\\0\\Issues.json",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\.sonarqube\\out\\0\\ProjectInfo.xml",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\bin\\Debug\\net7.0\\appsettings.Development.json",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\bin\\Debug\\net7.0\\appsettings.json",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\bin\\Debug\\net7.0\\CRA.deps.json",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\bin\\Debug\\net7.0\\CRA.runtimeconfig.json",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\bin\\Debug\\net7.0\\spa.proxy.json",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\node_modules\\abab\\index.d.ts",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\node_modules\\abab\\index.js",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\node_modules\\abab\\package.json",\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\node_modules\\accepts\\index.js",\
...
17407E65-4BE7-47B8-895A-C27074BD4BEE.sonar.cs.analyzer.projectOutPaths=\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\.sonarqube\\out\\0"
17407E65-4BE7-47B8-895A-C27074BD4BEE.sonar.cs.roslyn.reportFilePaths=\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\.sonarqube\\out\\0\\Issues.json"

17407E65-4BE7-47B8-895A-C27074BD4BEE.sonar.working.directory=C:\\Projects\\Sprints\\MultiFile\\CRA\\.sonarqube\\out\\.sonar\\mod0
sonar.host.url=http://localhost:9000
sonar.visualstudio.enable=false

sonar.modules=17407E65-4BE7-47B8-895A-C27074BD4BEE

ClientApp/src/App.test.js is only mentioned once in the sonar-project.properties but there seems to be confusion with respect to the combination of these settings

sonar.projectBaseDir=C:\\Projects\\Sprints\\MultiFile\\CRA
sonar.tests=\
"C:\\Projects\\Sprints\\MultiFile\\CRA\\ClientApp\\src\\App.test.js",\
17407E65-4BE7-47B8-895A-C27074BD4BEE.sonar.projectBaseDir=C:\\Projects\\Sprints\\MultiFile\\CRA
17407E65-4BE7-47B8-895A-C27074BD4BEE.sonar.sources=\

sonar.modules=17407E65-4BE7-47B8-895A-C27074BD4BEE

Maybe sonar.projectBaseDir and 17407E65-4BE7-47B8-895A-C27074BD4BEE.sonar.projectBaseDir should not be the same.

It works when I delete the App.test.js file (the only test file not excluded by SCM).

martin-strecker-sonarsource commented 
mkdir Unrooted
cd Unrooted
mkdir docs
cd docs
"Just a test" > Help.html
cd ..
dotnet new console --name ConsoleApp

# run the scanner version 7
dotnet Scanner7\SonarScanner.MSBuild.dll begin /k:"Unrooted" /d:sonar.host.url=http://localhost:9000 /sonar.token=squ_***
dotnet build .\ConsoleApp\ConsoleApp.csproj
dotnet Scanner7\SonarScanner.MSBuild.dll end /sonar.token=squ_***

# run the scanner version 8
dotnet Scanner8\SonarScanner.MSBuild.dll begin /k:"Unrooted" /d:sonar.host.url=http://localhost:9000 /sonar.token=squ_***
dotnet build .\ConsoleApp\ConsoleApp.csproj
dotnet Scanner8\SonarScanner.MSBuild.dll end /sonar.token=squ_***

The result on SQ looks like so:
Scanner 7:
image

Scanner 8:
image

The root directory changed, and we have one more file on the server (besides the .sonarqube folder, which is wrong).
Links to single issues created by scanner7, like http://localhost:9000/project/issues?open=2ce16516-46e7-4f26-9d7e-d86186ecc18f&id=Unrooted, still worked after this move.

gregory-paidis-sonarsource commented

Manually overridden sonar.languageXXX.file.suffixes from the server.

Changing sonar.javascript.file.suffixes to jjs picked up the new file as expected:

image

gregory-paidis-sonarsource commented

Test detection for .spec.js, .spec.ts, .test.js, test.ts, etc.

Success:
image

The files are correctly picked up and raise test-scoped rules, e.g.:

image

gregory-paidis-sonarsource commented

Disable MultiFileAnalysis with the flag

Works as expected, there are no root files picked up for js/ts/sql/etc.

CristianAmbrosini commented

Test UI Warning on local SQ/SC

The Warning was logged only when the full properties file path was invalid. I moved the logging position with #2129, and now it's correctly showing on SQ/SC. I added an additional test to validate the logs on the happy path as well.

image (2)

gregory-paidis-sonarsource commented

User-specified sonar.tests

This seems to break the analysis.

image

It is the same problem as noticed with the Create-react-app behavior, there are colissions between files, as they are indexed more than once.

I am not sure how often this behavior is, as our S4NET users do not usually specify the sonar.sources and sonar.tests parameters.

@martin-strecker-sonarsource Maybe we should show a warning if a user manually sets this parameter while multifile analysis is enabled? What do you think?

CristianAmbrosini commented

Test .esproj with Include/Exclude contents.

The Include and Exclude attribute used in a Script element within a .esproj is no longer respected by the automatic file detection. Users will have to manually set the sonar.exclusions property for these files.

vuewithaspbackend.esproj

<Project Sdk="Microsoft.VisualStudio.JavaScript.Sdk/0.5.85-alpha">
  <PropertyGroup Label="Globals">
    <ProjectGuid>ecf79eda-a781-4c86-a7ef-aeecdf00d792</ProjectGuid>
  </PropertyGroup>
  <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|AnyCPU'">
    <StartupCommand>npm run serve</StartupCommand>
    <JavaScriptTestRoot>.\</JavaScriptTestRoot>
    <JavaScriptTestFramework>Jest</JavaScriptTestFramework>
  </PropertyGroup>
  <ItemGroup>
    <Script Include="*.vue" Exclude="*.esproj;**\node_modules\**" />
  </ItemGroup>
  <Target Name="DebugEnsureNodeEnv" BeforeTargets="Build" Condition=" '$(Configuration)' == 'Debug' And !Exists('$(SpaRoot)node_modules') ">
    <Exec Command="node --version" ContinueOnError="true">
      <Output TaskParameter="ExitCode" PropertyName="ErrorCode" />
    </Exec>
    <Error Condition="'$(ErrorCode)' != '0'" Text="Node.js is required to build and run this project. To continue, please install Node.js from https://nodejs.org/, and then restart your command prompt or IDE." />
    <Message Importance="high" Text="Restoring dependencies using 'npm'. This may take several minutes..." />
    <Exec WorkingDirectory="$(SpaRoot)" Command="npm install" />
  </Target>
</Project>

Issues on .js files are raised even tho only .vue files are included.

CristianAmbrosini commented

Test the happy flow with the previous version of SQ

Test carried out with SQ 8.9.
The first analysis was done with the previous version of the scanner (old projectBaseDir detection) and found 1 bug.
The second analysis with the current master found 2 more bugs on .js and .ts files placed outside the solution folder, due to the projectBaseDir change, and the first bug remained tracked with a coherent history.

image image
martin-strecker-sonarsource commented

I couldn't find a scss or less project that comes with a cs backend (there are some less and scss files in angular dependencies but not in the scafolded code). I created a tailwind CSS-based next.js IT instead in #2138. Tailwind also does CSS magic behind the scenes but the PR showed that everything works as intended.

BUT

We need to activate the CSS plugin in the ITs to have CSS behavior under test. I did so in this commit 755e923 It should be cherry-picked from #2138 because the next.js itself PR doesn't bring anything new to the table (except the CSS plugin stuff). We should also add a regular CSS file in one of the checkMultiLanguageSupport.. tests so we do have at least one issue outside of node_modules in the ITs. I created #2141 to track this.

gregory-paidis-sonarsource commented

Test with the scm disabled

If SCM is disabled, there is a lot of noise, as seen on peach. The bin/ and obj/ directories can have a lot of xml files that are noisy.

gregory-paidis-sonarsource commented

Hidden directories starting with a . like .git .vs .rider and so on.

If the directories are under .gitignore, they are correctly ignored.
Otherwise, they are analyzed as normal files.

gregory-paidis-sonarsource commented

obj, bin, publish directories under .gitignore.

If a directory is .gitignored, it is properly not analyzed.
The only case that this is not true is if scm.disabled is set to true, where the .gitignore file is not respected.