Analyzers (#515)

* WIP * Add suppressions support * Apply suggestions * add more rules Update tests * add wildcard exclusions (similar to VS UI) * Update targets * Undo attempt at Targets * simplify * Update .Targets * Add IncludeAssets * Add comment
rr-wfm · Feb 26, 2024 · c14cf94 · c14cf94
1 parent 2f83fa3
commit c14cf94
Show file tree

Hide file tree

Showing 12 changed files with 312 additions and 3 deletions.
diff --git a/src/DacpacTool/BuildOptions.cs b/src/DacpacTool/BuildOptions.cs
@@ -18,6 +18,8 @@ public class BuildOptions : BaseOptions
         public FileInfo PostDeploy { get; set; }
         public FileInfo RefactorLog { get; set; }
 
+        public bool RunCodeAnalysis { get; set; }
+        public string CodeAnalysisRules { 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
@@ -12,6 +12,10 @@
   </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-preview3" IncludeAssets="runtime" />
+    <PackageReference Include="ErikEJ.DacFX.TSQLSmellSCA" Version="1.0.0-preview1" IncludeAssets="runtime" />
+
     <PackageReference Include="Microsoft.SqlServer.DacFx" Version="162.1.172" />
     <PackageReference Include="System.CommandLine" Version="2.0.0-beta4.22272.1" />
     <PackageReference Include="System.CommandLine.NamingConventionBinder" Version="2.0.0-beta4.22272.1" />

diff --git a/src/DacpacTool/ExtensibilityErrorExtensions.cs b/src/DacpacTool/ExtensibilityErrorExtensions.cs
@@ -0,0 +1,33 @@
+using System;
+using System.Text;
+using Microsoft.SqlServer.Dac.Extensibility;
+
+namespace MSBuild.Sdk.SqlProj.DacpacTool
+{
+    /// <summary>
+    /// A wrapper for <see cref="ExtensibilityError" /> that provides MSBuild compatible output and source document information.
+    /// </summary>
+    public static class ExtensibilityErrorExtensions
+    {
+        public static string GetOutputMessage(this ExtensibilityError extensibilityError)
+        {
+            ArgumentNullException.ThrowIfNull(extensibilityError);
+
+            var stringBuilder = new StringBuilder();
+            stringBuilder.Append(extensibilityError.Document);
+            stringBuilder.Append('(');
+            stringBuilder.Append(extensibilityError.Line);
+            stringBuilder.Append(',');
+            stringBuilder.Append(extensibilityError.Column);
+            stringBuilder.Append("):");
+            stringBuilder.Append(' ');
+            stringBuilder.Append(extensibilityError.Severity);
+            stringBuilder.Append(' ');
+            stringBuilder.Append(extensibilityError.ErrorCode);
+            stringBuilder.Append(": ");
+            stringBuilder.Append(extensibilityError.Message);
+
+            return stringBuilder.ToString();
+        }
+    }
+}
diff --git a/src/DacpacTool/PackageAnalyzer.cs b/src/DacpacTool/PackageAnalyzer.cs
@@ -0,0 +1,97 @@
+using System;
+using System.Collections.Generic;
+using System.IO;
+using Microsoft.SqlServer.Dac.CodeAnalysis;
+using Microsoft.SqlServer.Dac.Model;
+using System.Linq;
+
+namespace MSBuild.Sdk.SqlProj.DacpacTool
+{
+    public sealed class PackageAnalyzer
+    {
+        private readonly IConsole _console;
+        private readonly HashSet<string> _ignoredRules = new();
+        private readonly HashSet<string> _ignoredRuleSets = new();
+
+        public PackageAnalyzer(IConsole console, string rulesExpression)
+        {
+            _console = console ?? throw new ArgumentNullException(nameof(console));
+
+            BuildRuleLists(rulesExpression);
+        }
+
+        public void Analyze(TSqlModel model, FileInfo outputFile)
+        {
+            _console.WriteLine($"Analyzing package '{outputFile.FullName}'");
+            try
+            {
+                var factory = new CodeAnalysisServiceFactory();
+                var service = factory.CreateAnalysisService(model);
+
+                if (_ignoredRules.Count > 0 || _ignoredRuleSets.Count > 0)
+                {
+                    service.SetProblemSuppressor(p => 
+                        _ignoredRules.Contains(p.Rule.RuleId) 
+                        || _ignoredRuleSets.Any(s => p.Rule.RuleId.StartsWith(s)));
+                }
+
+                var result = service.Analyze(model);
+                if (!result.AnalysisSucceeded)
+                {
+                    var errors = result.GetAllErrors();
+                    foreach (var err in errors)
+                    {
+                        _console.WriteLine(err.GetOutputMessage());
+                    }
+                    return;
+                }
+                else
+                {
+                    foreach (var err in result.Problems)
+                    {
+                        _console.WriteLine(err.GetOutputMessage());
+                    }
+
+                    result.SerializeResultsToXml(GetOutputFileName(outputFile));
+                }
+                _console.WriteLine($"Successfully analyzed package '{outputFile}'");
+            }
+            catch (Exception ex)
+            {
+                _console.WriteLine($"ERROR: An unknown error occurred while analyzing package '{outputFile.FullName}': {ex.Message}");
+            }
+        }
+
+        private void BuildRuleLists(string rulesExpression)
+        {
+            if (!string.IsNullOrWhiteSpace(rulesExpression))
+            {
+                foreach (var rule in rulesExpression.Split(new[] { ';' },
+                    StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries)
+                        .Where(rule => rule
+                            .StartsWith("-", StringComparison.OrdinalIgnoreCase)
+                                && rule.Length > 1))
+                {
+                    if (rule.EndsWith("*", StringComparison.OrdinalIgnoreCase))
+                    {
+                        _ignoredRuleSets.Add(rule[1..^1]);
+                    }
+                    else
+                    {
+                        _ignoredRules.Add(rule[1..]);
+                    }
+                }
+            }
+        }
+
+        private static string GetOutputFileName(FileInfo outputFile)
+        {
+            var outputFileName = outputFile.FullName;
+            if (outputFile.Extension.Equals(".dacpac", StringComparison.OrdinalIgnoreCase))
+            {
+                outputFileName = outputFile.FullName[..^7];
+            }
+            return outputFileName + ".CodeAnalysis.xml";
+        }
+    }
+}
diff --git a/src/DacpacTool/Program.cs b/src/DacpacTool/Program.cs
@@ -29,6 +29,9 @@ static async Task<int> Main(string[] args)
                 new Option<string[]>(new string[] { "--buildproperty", "-bp" }, "Build properties to be set on the model"),
                 new Option<string[]>(new string[] { "--deployproperty", "-dp" }, "Deploy properties to be set for the create script"),
                 new Option<string[]>(new string[] { "--sqlcmdvar", "-sc" }, "SqlCmdVariable(s) to include"),
+
+                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<bool>(new string[] { "--warnaserror" }, "Treat T-SQL Warnings As Errors"),
                 new Option<bool>(new string[] { "--generatecreatescript", "-gcs" }, "Generate create script for package"),
@@ -191,6 +194,12 @@ private static int BuildDacpac(BuildOptions options)
                 packageBuilder.GenerateCreateScript(options.Output, options.TargetDatabaseName ?? options.Name, deployOptions);
             }
 
+            if (options.RunCodeAnalysis)
+            {
+                var analyzer = new PackageAnalyzer(new ActualConsole(), options.CodeAnalysisRules);
+                analyzer.Analyze(packageBuilder.Model, options.Output);
+            }
+
             return 0;
         }
 

diff --git a/src/DacpacTool/SqlRuleProblemExtensions.cs b/src/DacpacTool/SqlRuleProblemExtensions.cs
@@ -0,0 +1,31 @@
+using System;
+using System.Text;
+using Microsoft.SqlServer.Dac.CodeAnalysis;
+
+namespace MSBuild.Sdk.SqlProj.DacpacTool
+{
+    /// <summary>
+    /// A wrapper for <see cref="SqlRuleProblem" /> that provides MSBuild compatible output and source document information.
+    /// </summary>
+    public static class SqlRuleProblemExtensions
+    {
+        public static string GetOutputMessage(this SqlRuleProblem sqlRuleProblem)
+        {
+            ArgumentNullException.ThrowIfNull(sqlRuleProblem);
+
+            var stringBuilder = new StringBuilder();
+            stringBuilder.Append(sqlRuleProblem.SourceName);
+            stringBuilder.Append('(');
+            stringBuilder.Append(sqlRuleProblem.StartLine);
+            stringBuilder.Append(',');
+            stringBuilder.Append(sqlRuleProblem.StartColumn);
+            stringBuilder.Append("):");
+            stringBuilder.Append(' ');
+            stringBuilder.Append(sqlRuleProblem.Severity);
+            stringBuilder.Append(' ');
+            stringBuilder.Append(sqlRuleProblem.ErrorMessageString);
+
+            return stringBuilder.ToString();
+        }
+    }
+}
diff --git a/src/MSBuild.Sdk.SqlProj/Sdk/Sdk.targets b/src/MSBuild.Sdk.SqlProj/Sdk/Sdk.targets
@@ -229,13 +229,15 @@
       <PreDeploymentScriptArgument>@(PreDeploy->'--predeploy %(Identity)', ' ')</PreDeploymentScriptArgument>
       <PostDeploymentScriptArgument>@(PostDeploy->'--postdeploy %(Identity)', ' ')</PostDeploymentScriptArgument>
       <RefactorLogScriptArgument>@(RefactorLog->'--refactorlog %(Identity)', ' ')</RefactorLogScriptArgument>
+      <RunSqlCodeAnalysisArgument Condition="'$(RunSqlCodeAnalysis)' == 'True'">-an</RunSqlCodeAnalysisArgument>
+      <CodeAnalysisRulesArgument Condition="'$(CodeAnalysisRules)'!=''">-ar $(CodeAnalysisRules)</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)</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)</DacpacToolCommand>
     </PropertyGroup>
     <!-- Run it, except during design-time builds -->
     <Message Importance="Low" Text="Running command: $(DacpacToolCommand)" />

diff --git a/test/DacpacTool.Tests/PackageAnalyzerTests.cs b/test/DacpacTool.Tests/PackageAnalyzerTests.cs
@@ -0,0 +1,87 @@
+using System.IO;
+using Microsoft.SqlServer.Dac.Model;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using Shouldly;
+
+namespace MSBuild.Sdk.SqlProj.DacpacTool.Tests
+{
+    [TestClass]
+    public class PackageAnalyzerTests
+    {
+        private readonly IConsole _console = new TestConsole();
+
+        [TestMethod]
+        public void RunsAnalyzer()
+        {
+            // Arrange
+            var testConsole = (TestConsole)_console;
+            testConsole.Lines.Clear();
+            var result = BuildSimpleModel();
+            var packageAnalyzer = new PackageAnalyzer(_console, null);
+
+            // Act
+            packageAnalyzer.Analyze(result.model, result.fileInfo);
+
+            // Assert
+            testConsole.Lines.Count.ShouldBe(15);
+
+            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()
+        {
+            // Arrange
+            var testConsole = (TestConsole)_console;
+            testConsole.Lines.Clear();
+            var result = BuildSimpleModel();
+            var packageAnalyzer = new PackageAnalyzer(_console, "-SqlServer.Rules.SRD0006;-Smells.SML005;-SqlServer.Rules.SRD999;;");
+
+            // Act
+            packageAnalyzer.Analyze(result.model, result.fileInfo);
+
+            // Assert
+            testConsole.Lines.Count.ShouldBe(13);
+
+            testConsole.Lines.ShouldContain($"Analyzing package '{result.fileInfo.FullName}'");
+            testConsole.Lines.ShouldNotContain($"SRD0006");
+            testConsole.Lines.ShouldNotContain($"SML005");
+            testConsole.Lines.ShouldContain($"Successfully analyzed package '{result.fileInfo.FullName}'");
+        }
+
+        [TestMethod]
+        public void RunsAnalyzerWithWildcardSupressions()
+        {
+            // Arrange
+            var testConsole = (TestConsole)_console;
+            testConsole.Lines.Clear();
+            var result = BuildSimpleModel();
+            var packageAnalyzer = new PackageAnalyzer(_console, "-SqlServer.Rules.SRD*");
+
+            // Act
+            packageAnalyzer.Analyze(result.model, result.fileInfo);
+
+            // Assert
+            testConsole.Lines.Count.ShouldBe(13);
+
+            testConsole.Lines.ShouldContain($"Analyzing package '{result.fileInfo.FullName}'");
+            testConsole.Lines.ShouldNotContain($"SRD");
+            testConsole.Lines.ShouldContain($"Successfully analyzed package '{result.fileInfo.FullName}'");
+        }
+
+        private static (FileInfo fileInfo, TSqlModel model) BuildSimpleModel()
+        {
+            var tmodel = new TestModelBuilder()
+                .AddTable("TestTable", ("Column1", "nvarchar(100)"))
+                .AddStoredProcedure("sp_GetData", "SELECT * FROM dbo.TestTable", "proc1.sql");
+
+            var model = tmodel.Build();
+            var packagePath = tmodel.SaveAsPackage();
+
+            return (new FileInfo(packagePath), model);
+        }
+    }
+}
diff --git a/test/DacpacTool.Tests/TestConsole.cs b/test/DacpacTool.Tests/TestConsole.cs
@@ -0,0 +1,20 @@
+using System;
+using System.Collections.Generic;
+
+namespace MSBuild.Sdk.SqlProj.DacpacTool.Tests
+{
+    internal class TestConsole : IConsole
+    {
+        public readonly List<string> Lines = new List<string>();
+
+        public string ReadLine()
+        {
+            throw new NotImplementedException();
+        }
+
+        public void WriteLine(string value)
+        {
+            Lines.Add(value);
+        }
+    }
+}
diff --git a/test/DacpacTool.Tests/TestModelBuilder.cs b/test/DacpacTool.Tests/TestModelBuilder.cs
@@ -28,10 +28,17 @@ public TestModelBuilder AddTable(string tableName, params (string name, string t
             return this;
         }
 
-        public TestModelBuilder AddStoredProcedure(string procName, string body)
+        public TestModelBuilder AddStoredProcedure(string procName, string body, string fileName = null)
         {
             var procDefinition = $"CREATE PROCEDURE [{procName}] AS BEGIN {body} END";
-            sqlModel.AddObjects(procDefinition);
+            if (!string.IsNullOrEmpty(fileName))
+            {
+                sqlModel.AddOrUpdateObjects(procDefinition, fileName, new TSqlObjectOptions());
+            }
+            else
+            {
+                sqlModel.AddObjects(procDefinition);
+            }
             return this;
         }
 

diff --git a/test/TestProjectWithAnalyzers/Procedures/sp_Test.sql b/test/TestProjectWithAnalyzers/Procedures/sp_Test.sql
@@ -0,0 +1,5 @@
+CREATE PROCEDURE [dbo].[sp_Test]
+AS
+BEGIN
+    SELECT * FROM [dbo].[MyTable];
+END
diff --git a/test/TestProjectWithAnalyzers/TestProjectWithAnalyzers.csproj b/test/TestProjectWithAnalyzers/TestProjectWithAnalyzers.csproj
@@ -0,0 +1,12 @@
+<Project>
+  <Import Project="$(MSBuildThisFileDirectory)..\..\src\MSBuild.Sdk.SqlProj\Sdk\Sdk.props" />
+
+  <PropertyGroup>
+    <TargetFramework>netstandard2.0</TargetFramework>
+    <SqlServerVersion>Sql150</SqlServerVersion>
+    <RunSqlCodeAnalysis>True</RunSqlCodeAnalysis>
+    <CodeAnalysisRules>-SqlServer.Rules.SRD0006;-Smells.*</CodeAnalysisRules>      
+  </PropertyGroup>
+
+  <Import Project="$(MSBuildThisFileDirectory)..\..\src\MSBuild.Sdk.SqlProj\Sdk\Sdk.targets" />
+</Project>