Support NuGet based code analyzers (#625)

* Support NuGet based code analyzers * Move 3rd party analyzser packages to template * fix tests * Update docs Remove file based "bring your own rules" support * Log analyzer load and warn if none * Update template to netstandard2.1 * Update README.md Co-authored-by: Jeff Rosenberg <[email protected]> * Update README.md Co-authored-by: Jeff Rosenberg <[email protected]> * Update README.md Co-authored-by: Jeff Rosenberg <[email protected]> * Update readme * fix up * Clean up * fix test * Address review comments * Always enable code analysis * revert --------- Co-authored-by: Jeff Rosenberg <[email protected]>
rr-wfm · Oct 1, 2024 · f7d9b0a · f7d9b0a
1 parent 1f7ebda
commit f7d9b0a
Show file tree

Hide file tree

Showing 15 changed files with 144 additions and 107 deletions.
diff --git a/README.md b/README.md
diff --git a/src/DacpacTool/BuildOptions.cs b/src/DacpacTool/BuildOptions.cs
@@ -22,6 +22,7 @@ public class BuildOptions : BaseOptions
 
         public bool RunCodeAnalysis { get; set; }
         public string CodeAnalysisRules { get; set; }
+        public FileInfo[] CodeAnalysisAssemblies { get; set; }
         public bool WarnAsError { get; set; }
         public string SuppressWarnings { get; set; }
         public FileInfo SuppressWarningsListFile { get; set; }

diff --git a/src/DacpacTool/DacpacTool.csproj b/src/DacpacTool/DacpacTool.csproj
@@ -16,11 +16,7 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <!-- These packages contain DacFX analysis rules, which must be located with the executable in order to be discovered-->
-    <PackageReference Include="ErikEJ.DacFX.SqlServer.Rules" Version="1.0.0" IncludeAssets="runtime" />
-    <PackageReference Include="ErikEJ.DacFX.TSQLSmellSCA" Version="1.0.0" IncludeAssets="runtime" />
-
-    <PackageReference Include="Microsoft.SqlServer.DacFx" Version="162.3.566" />
+    <PackageReference Include="Microsoft.SqlServer.DacFx" Version="162.4.92" />
     <PackageReference Include="System.CommandLine" Version="2.0.0-beta4.22272.1" />
     <PackageReference Include="System.CommandLine.NamingConventionBinder" Version="2.0.0-beta4.22272.1" />
   </ItemGroup>

diff --git a/src/DacpacTool/PackageAnalyzer.cs b/src/DacpacTool/PackageAnalyzer.cs
@@ -22,33 +22,29 @@ public PackageAnalyzer(IConsole console, string rulesExpression)
             BuildRuleLists(rulesExpression);
         }
 
-        public void AddRulesFile(FileInfo inputFile)
-        {
-            ArgumentNullException.ThrowIfNull(inputFile);
-
-            // Make sure the file exists
-            if (!inputFile.Exists)
-            {
-                throw new ArgumentException($"Unable to find rules file {inputFile}", nameof(inputFile));
-            }
-
-            if (inputFile.Directory.Name.Equals("rules", StringComparison.OrdinalIgnoreCase)
-                && inputFile.Extension.EndsWith(".dll", StringComparison.OrdinalIgnoreCase))
-            {
-                CopyAdditionalRulesFile(inputFile);
-            }
-        }
-
-        public void Analyze(TSqlModel model, FileInfo outputFile)
+        public void Analyze(TSqlModel model, FileInfo outputFile, FileInfo[] analyzers)
         {
             ArgumentNullException.ThrowIfNull(model);
             ArgumentNullException.ThrowIfNull(outputFile);
+            ArgumentNullException.ThrowIfNull(analyzers);
 
             _console.WriteLine($"Analyzing package '{outputFile.FullName}'");
             try
             {
                 var factory = new CodeAnalysisServiceFactory();
-                var service = factory.CreateAnalysisService(model);
+                var settings = new CodeAnalysisServiceSettings();
+
+                if (analyzers.Length == 0)
+                {
+                    _console.WriteLine("DacpacTool warning SQLPROJ0001: No additional rules files found, consider adding more rules via PackageReference - see the readme here: https://github.com/rr-wfm/MSBuild.Sdk.SqlProj.");
+                }
+                else
+                {
+                    _console.WriteLine("Using additional analyzers: " + string.Join(", ", analyzers.Select(a => a.FullName)));
+                    settings.AssemblyLookupPath = string.Join(';', analyzers.Select(a => a.DirectoryName));
+                }
+
+                var service = factory.CreateAnalysisService(model, settings);
 
                 if (_ignoredRules.Count > 0 || _ignoredRuleSets.Count > 0)
                 {
@@ -125,16 +121,5 @@ private static string GetOutputFileName(FileInfo outputFile)
             }
             return outputFileName + ".CodeAnalysis.xml";
         }
-
-        private void CopyAdditionalRulesFile(FileInfo rulesFile)
-        {
-            var destPath = Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().Location);
-
-            var dest = Path.Combine(destPath, rulesFile.Name);
-
-            rulesFile.CopyTo(dest, overwrite: true);
-
-            _console.WriteLine($"Adding additional rules file from '{rulesFile.FullName}' to '{dest}'");
-        }
     }
 }
diff --git a/src/DacpacTool/PackageBuilder.cs b/src/DacpacTool/PackageBuilder.cs
@@ -78,13 +78,6 @@ public bool AddInputFile(FileInfo inputFile)
                 throw new ArgumentException($"Unable to find input file {inputFile}", nameof(inputFile));
             }
 
-            // Skip custom rules files, they will be added to the tools folder later by the analyzer
-            if (inputFile.Directory.Name.Equals("rules", StringComparison.OrdinalIgnoreCase)
-                && inputFile.Extension.EndsWith(".dll", StringComparison.OrdinalIgnoreCase))
-            {
-                return true;
-            }
-
             _console.WriteLine($"Adding {inputFile.FullName} to the model");
 
             try

diff --git a/src/DacpacTool/Program.cs b/src/DacpacTool/Program.cs
@@ -33,6 +33,7 @@ static async Task<int> Main(string[] args)
 
                 new Option<bool>(new string[] { "--runcodeanalysis", "-an" }, "Run static code analysis"),
                 new Option<string>(new string[] { "--codeanalysisrules", "-ar" }, "List of rules to suppress in format '-Microsoft.Rules.Data.SR0001;-Microsoft.Rules.Data.SR0008'"),
+                new Option<FileInfo[]>(new string[] { "--codeanalysisassemblies", "-aa" }, "Custom code analysis rule assemblies to use"),
 
                 new Option<bool>(new string[] { "--warnaserror" }, "Treat T-SQL Warnings As Errors"),
                 new Option<bool>(new string[] { "--generatecreatescript", "-gcs" }, "Generate create script for package"),
@@ -206,13 +207,7 @@ private static int BuildDacpac(BuildOptions options)
             {
                 var analyzer = new PackageAnalyzer(new ActualConsole(), options.CodeAnalysisRules);
 
-                foreach (var line in File.ReadLines(options.InputFile.FullName))
-                {
-                    FileInfo inputFile = new FileInfo(line); // Validation occurs in AddRulesFile
-                    analyzer.AddRulesFile(inputFile);
-                }
-
-                analyzer.Analyze(packageBuilder.Model, options.Output);
+                analyzer.Analyze(packageBuilder.Model, options.Output, options.CodeAnalysisAssemblies ?? Array.Empty<FileInfo>());
             }
 
             return 0;

diff --git a/src/MSBuild.Sdk.SqlProj.Templates/templates/sqlproj/sqlproj.csproj b/src/MSBuild.Sdk.SqlProj.Templates/templates/sqlproj/sqlproj.csproj
@@ -1,11 +1,18 @@
 <Project Sdk="MSBuild.Sdk.SqlProj/#{NBGV_NuGetPackageVersion}#">
     <PropertyGroup>
-        <TargetFramework>netstandard2.0</TargetFramework>
+        <TargetFramework>netstandard2.1</TargetFramework>
         <SqlServerVersion>#{SqlServerVersion}</SqlServerVersion>
         <RunSqlCodeAnalysis>#{CodeAnalysis}</RunSqlCodeAnalysis>
         <!-- For additional properties that can be set here, please refer to https://github.com/rr-wfm/MSBuild.Sdk.SqlProj#model-properties -->
     </PropertyGroup>
 
+    <ItemGroup>
+        <!-- These packages adds additional code analysis rules -->
+        <!-- We recommend using these, but they can be removed if desired -->
+        <PackageReference Include="ErikEJ.DacFX.SqlServer.Rules" Version="1.1.0" />
+        <PackageReference Include="ErikEJ.DacFX.TSQLSmellSCA" Version="1.1.0" />
+    </ItemGroup>
+
     <PropertyGroup>
         <!-- Refer to https://github.com/rr-wfm/MSBuild.Sdk.SqlProj#publishing-support for supported publishing options -->
     </PropertyGroup>

diff --git a/src/MSBuild.Sdk.SqlProj/Sdk/Sdk.targets b/src/MSBuild.Sdk.SqlProj/Sdk/Sdk.targets
@@ -229,14 +229,15 @@
       <PostDeploymentScriptArgument>@(PostDeploy->'--postdeploy %(Identity)', ' ')</PostDeploymentScriptArgument>
       <RefactorLogScriptArgument>@(RefactorLog->'--refactorlog %(Identity)', ' ')</RefactorLogScriptArgument>
       <RunSqlCodeAnalysisArgument Condition="'$(RunSqlCodeAnalysis)' == 'True'">-an</RunSqlCodeAnalysisArgument>
+      <CodeAnalysisAssemblyLookupPathsArgument Condition="'$(RunSqlCodeAnalysis)' == 'True'">@(Analyzer->'-aa &quot;%(Identity)&quot;', ' ')</CodeAnalysisAssemblyLookupPathsArgument>
       <CodeAnalysisRulesArgument Condition="'$(CodeAnalysisRules)'!=''">-ar &quot;$(CodeAnalysisRules)&quot;</CodeAnalysisRulesArgument>
       <DebugArgument Condition="'$(MSBuildSdkSqlProjDebug)' == 'True'">--debug</DebugArgument>
       <TreatTSqlWarningsAsErrorsArgument Condition="'$(TreatTSqlWarningsAsErrors)' == 'True' Or ('$(TreatWarningsAsErrors)' == 'True' And '$(TreatTSqlWarningsAsErrors)' == '')">--warnaserror</TreatTSqlWarningsAsErrorsArgument>
       <GenerateCreateScriptArgument Condition="'$(GenerateCreateScript)' == 'True'">--generatecreatescript</GenerateCreateScriptArgument>
       <TargetDatabaseNameArgument Condition="'$(TargetDatabaseName)' != '$(MSBuildProjectName)'">-tdn &quot;$(TargetDatabaseName)&quot;</TargetDatabaseNameArgument>
       <SuppressTSqlWarningsArgument Condition="'$(SuppressTSqlWarnings)'!=''">-spw &quot;$(SuppressTSqlWarnings)&quot;</SuppressTSqlWarningsArgument>
       <WarningsSuppressionListArgument Condition="'@(WarningsSuppressionFiles->'%(Identity)')'!=''">-spl &quot;$(IntermediateOutputPath)$(MSBuildProjectName).WarningsSuppression.txt&quot;</WarningsSuppressionListArgument>
-      <DacpacToolCommand>dotnet &quot;$(DacpacToolExe)&quot; build $(OutputPathArgument) $(MetadataArguments) $(SqlServerVersionArgument) $(InputFileArguments) $(ReferenceArguments) $(SqlCmdVariableArguments) $(BuildPropertyArguments) $(DeployPropertyArguments) $(PreDeploymentScriptArgument) $(PostDeploymentScriptArgument) $(RefactorLogScriptArgument) $(TreatTSqlWarningsAsErrorsArgument) $(SuppressTSqlWarningsArgument) $(WarningsSuppressionListArgument) $(DebugArgument) $(GenerateCreateScriptArgument) $(TargetDatabaseNameArgument) $(RunSqlCodeAnalysisArgument) $(CodeAnalysisRulesArgument)</DacpacToolCommand>
+      <DacpacToolCommand>dotnet &quot;$(DacpacToolExe)&quot; build $(OutputPathArgument) $(MetadataArguments) $(SqlServerVersionArgument) $(InputFileArguments) $(ReferenceArguments) $(SqlCmdVariableArguments) $(BuildPropertyArguments) $(DeployPropertyArguments) $(PreDeploymentScriptArgument) $(PostDeploymentScriptArgument) $(RefactorLogScriptArgument) $(TreatTSqlWarningsAsErrorsArgument) $(SuppressTSqlWarningsArgument) $(WarningsSuppressionListArgument) $(DebugArgument) $(GenerateCreateScriptArgument) $(TargetDatabaseNameArgument) $(RunSqlCodeAnalysisArgument) $(CodeAnalysisRulesArgument) $(CodeAnalysisAssemblyLookupPathsArgument)</DacpacToolCommand>
     </PropertyGroup>
     <!-- Run it, except during design-time builds -->
     <Message Importance="Low" Text="Running command: $(DacpacToolCommand)" />

diff --git a/test/DacpacTool.Tests/DacpacTool.Tests.csproj b/test/DacpacTool.Tests/DacpacTool.Tests.csproj
@@ -8,6 +8,18 @@
     <LangVersion>9.0</LangVersion>
   </PropertyGroup>
 
+  <ItemGroup>
+    <Content Include="SqlServer.Dac.dll">
+      <CopyToOutputDirectory>Always</CopyToOutputDirectory>
+    </Content>
+    <Content Include="SqlServer.Rules.dll">
+      <CopyToOutputDirectory>Always</CopyToOutputDirectory>
+    </Content>
+    <Content Include="TSQLSmellSCA.dll">
+      <CopyToOutputDirectory>Always</CopyToOutputDirectory>
+    </Content>
+  </ItemGroup>
+
   <ItemGroup>
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.10.0" />
     <PackageReference Include="MSTest.TestAdapter" Version="3.6.0" />

diff --git a/test/DacpacTool.Tests/PackageAnalyzerTests.cs b/test/DacpacTool.Tests/PackageAnalyzerTests.cs
@@ -1,4 +1,6 @@
-using System.IO;
+using System;
+using System.Collections.Generic;
+using System.IO;
 using System.Linq;
 using Microsoft.SqlServer.Dac.Model;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
@@ -21,17 +23,19 @@ public void RunsAnalyzer()
             var packageAnalyzer = new PackageAnalyzer(_console, null);
 
             // Act
-            packageAnalyzer.Analyze(result.model, result.fileInfo);
+            packageAnalyzer.Analyze(result.model, result.fileInfo, CollectAssemblyPaths());
 
             // Assert
-            testConsole.Lines.Count.ShouldBe(15);
+            testConsole.Lines.Count.ShouldBe(16);
 
             testConsole.Lines.ShouldContain($"Analyzing package '{result.fileInfo.FullName}'");
             testConsole.Lines.ShouldContain($"proc1.sql(1,47): Warning SRD0006 : SqlServer.Rules : Avoid using SELECT *.");
             testConsole.Lines.ShouldContain($"proc1.sql(1,47): Warning SML005 : Smells : Avoid use of 'Select *'");
             testConsole.Lines.ShouldContain($"Successfully analyzed package '{result.fileInfo.FullName}'");
         }
 
+
+
         [TestMethod]
         public void RunsAnalyzerWithSupressions()
         {
@@ -42,10 +46,10 @@ public void RunsAnalyzerWithSupressions()
             var packageAnalyzer = new PackageAnalyzer(_console, "-SqlServer.Rules.SRD0006;-Smells.SML005;-SqlServer.Rules.SRD999;+!SqlServer.Rules.SRN0002;");
 
             // Act
-            packageAnalyzer.Analyze(result.model, result.fileInfo);
+            packageAnalyzer.Analyze(result.model, result.fileInfo, CollectAssemblyPaths());
 
             // Assert
-            testConsole.Lines.Count.ShouldBe(13);
+            testConsole.Lines.Count.ShouldBe(14);
 
             testConsole.Lines.ShouldContain($"Analyzing package '{result.fileInfo.FullName}'");
             testConsole.Lines.Any(l => l.Contains("SRD0006")).ShouldBeFalse();
@@ -64,10 +68,10 @@ public void RunsAnalyzerWithWildcardSupressions()
             var packageAnalyzer = new PackageAnalyzer(_console, "-SqlServer.Rules.SRD*");
 
             // Act
-            packageAnalyzer.Analyze(result.model, result.fileInfo);
+            packageAnalyzer.Analyze(result.model, result.fileInfo, CollectAssemblyPaths());
 
             // Assert
-            testConsole.Lines.Count.ShouldBe(13);
+            testConsole.Lines.Count.ShouldBe(14);
 
             testConsole.Lines.ShouldContain($"Analyzing package '{result.fileInfo.FullName}'");
             testConsole.Lines.Any(l => l.Contains("SRD")).ShouldBeFalse();
@@ -84,10 +88,10 @@ public void RunsAnalyzerWithWarningsAsErrors()
             var packageAnalyzer = new PackageAnalyzer(_console, "+!SqlServer.Rules.SRD0006");
 
             // Act
-            packageAnalyzer.Analyze(result.model, result.fileInfo);
+            packageAnalyzer.Analyze(result.model, result.fileInfo, CollectAssemblyPaths());
 
             // Assert
-            testConsole.Lines.Count.ShouldBe(15);
+            testConsole.Lines.Count.ShouldBe(16);
 
             testConsole.Lines.ShouldContain($"Analyzing package '{result.fileInfo.FullName}'");
             testConsole.Lines.ShouldContain($"proc1.sql(1,47): Error SRD0006 : SqlServer.Rules : Avoid using SELECT *.");
@@ -105,18 +109,41 @@ public void RunsAnalyzerWithWarningsAsErrorsUsingWildcard()
             var packageAnalyzer = new PackageAnalyzer(_console, "+!SqlServer.Rules.SRD*");
 
             // Act
-            packageAnalyzer.Analyze(result.model, result.fileInfo);
+            packageAnalyzer.Analyze(result.model, result.fileInfo, CollectAssemblyPaths());
 
             // Assert
-            testConsole.Lines.Count.ShouldBe(15);
+            testConsole.Lines.Count.ShouldBe(16);
 
+            testConsole.Lines.Count(l => l.Contains("Using additional analyzers: ")).ShouldBe(1);
             testConsole.Lines.ShouldContain($"Analyzing package '{result.fileInfo.FullName}'");
+            testConsole.Lines.ShouldNotContain("DacpacTool warning SQLPROJ0001: No additional rules files found, consider adding more rules via PackageReference - see the readme here: https://github.com/rr-wfm/MSBuild.Sdk.SqlProj.");
             testConsole.Lines.ShouldContain($"proc1.sql(1,47): Error SRD0006 : SqlServer.Rules : Avoid using SELECT *.");
             testConsole.Lines.ShouldContain($"-1(1,1): Error SRD0002 : SqlServer.Rules : Table does not have a primary key.");
             testConsole.Lines.Count(l => l.Contains("): Error ")).ShouldBe(2);
             testConsole.Lines.ShouldContain($"Successfully analyzed package '{result.fileInfo.FullName}'");
         }
 
+        [TestMethod]
+        public void RunsAnalyzerWithoutAdditionalAnalyzers()
+        {
+            // Arrange
+            var testConsole = (TestConsole)_console;
+            testConsole.Lines.Clear();
+            var result = BuildSimpleModel();
+            var packageAnalyzer = new PackageAnalyzer(_console, null);
+
+            // Act
+            packageAnalyzer.Analyze(result.model, result.fileInfo, Array.Empty<FileInfo>());
+
+            // Assert
+            testConsole.Lines.Count.ShouldBe(16);
+
+            testConsole.Lines[1].ShouldBe("DacpacTool warning SQLPROJ0001: No additional rules files found, consider adding more rules via PackageReference - see the readme here: https://github.com/rr-wfm/MSBuild.Sdk.SqlProj.");
+            testConsole.Lines.ShouldContain($"Analyzing package '{result.fileInfo.FullName}'");
+            testConsole.Lines.Count(l => l.Contains("): Error ")).ShouldBe(0);
+            testConsole.Lines.ShouldContain($"Successfully analyzed package '{result.fileInfo.FullName}'");
+        }
+
         private static (FileInfo fileInfo, TSqlModel model) BuildSimpleModel()
         {
             var tmodel = new TestModelBuilder()
@@ -128,5 +155,15 @@ private static (FileInfo fileInfo, TSqlModel model) BuildSimpleModel()
 
             return (new FileInfo(packagePath), model);
         }
+
+        private FileInfo[] CollectAssemblyPaths()
+        {
+            var result = new List<FileInfo>();
+            var path = Path.GetDirectoryName(Path.Combine(System.Reflection.Assembly.GetAssembly(typeof(PackageAnalyzerTests)).Location));
+            result.Add(new FileInfo(Path.Combine(path, "SqlServer.Rules.dll")));
+            result.Add(new FileInfo(Path.Combine(path, "TSQLSmellSCA.dll")));
+
+            return result.ToArray();
+        }
     }
 }
diff --git a/test/DacpacTool.Tests/SqlServer.Dac.dll b/test/DacpacTool.Tests/SqlServer.Dac.dll
diff --git a/...ctWithAnalyzers/Rules/SqlServer.Rules.dll → test/DacpacTool.Tests/SqlServer.Rules.dll b/...ctWithAnalyzers/Rules/SqlServer.Rules.dll → test/DacpacTool.Tests/SqlServer.Rules.dll
diff --git a/test/DacpacTool.Tests/TSQLSmellSCA.dll b/test/DacpacTool.Tests/TSQLSmellSCA.dll
diff --git a/test/TestProjectWithAnalyzers/Rules/SqlServer.Dac.dll b/test/TestProjectWithAnalyzers/Rules/SqlServer.Dac.dll
diff --git a/test/TestProjectWithAnalyzers/TestProjectWithAnalyzers.csproj b/test/TestProjectWithAnalyzers/TestProjectWithAnalyzers.csproj
@@ -2,16 +2,15 @@
   <Import Project="$(MSBuildThisFileDirectory)..\..\src\MSBuild.Sdk.SqlProj\Sdk\Sdk.props" />
 
   <PropertyGroup>
-    <TargetFramework>netstandard2.0</TargetFramework>
+    <TargetFramework>netstandard2.1</TargetFramework>
     <SqlServerVersion>Sql150</SqlServerVersion>
-    <RunSqlCodeAnalysis>True</RunSqlCodeAnalysis>
     <CodeAnalysisRules>-SqlServer.Rules.SRD0006;-Smells.*;+!SqlServer.Rules.SRN0002</CodeAnalysisRules>
+    <RunSqlCodeAnalysis>true</RunSqlCodeAnalysis>
   </PropertyGroup>
 
-  <Import Project="$(MSBuildThisFileDirectory)..\..\src\MSBuild.Sdk.SqlProj\Sdk\Sdk.targets" />
-
   <ItemGroup>
-    <Content Include="Rules\SqlServer.Dac.dll" />
-    <Content Include="Rules\SqlServer.Rules.dll" />
+      <PackageReference Include="ErikEJ.DacFX.SqlServer.Rules" Version="1.1.0" />
   </ItemGroup>
+
+  <Import Project="$(MSBuildThisFileDirectory)..\..\src\MSBuild.Sdk.SqlProj\Sdk\Sdk.targets" />
 </Project>