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

Support NuGet based code analyzers #625

Merged
merged 17 commits into from
Oct 1, 2024
Merged
89 changes: 50 additions & 39 deletions README.md

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions src/DacpacTool/BuildOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down
6 changes: 1 addition & 5 deletions src/DacpacTool/DacpacTool.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
45 changes: 15 additions & 30 deletions src/DacpacTool/PackageAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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}'");
}
}
}
7 changes: 0 additions & 7 deletions src/DacpacTool/PackageBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 2 additions & 7 deletions src/DacpacTool/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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>
ErikEJ marked this conversation as resolved.
Show resolved Hide resolved
<!-- 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 -->
ErikEJ marked this conversation as resolved.
Show resolved Hide resolved
<!-- 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>
Expand Down
3 changes: 2 additions & 1 deletion src/MSBuild.Sdk.SqlProj/Sdk/Sdk.targets
Original file line number Diff line number Diff line change
Expand Up @@ -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)" />
Expand Down
12 changes: 12 additions & 0 deletions test/DacpacTool.Tests/DacpacTool.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down
59 changes: 48 additions & 11 deletions test/DacpacTool.Tests/PackageAnalyzerTests.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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()
{
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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 *.");
Expand All @@ -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()
Expand All @@ -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();
}
}
}
Binary file added test/DacpacTool.Tests/SqlServer.Dac.dll
Binary file not shown.
Binary file not shown.
Binary file added test/DacpacTool.Tests/TSQLSmellSCA.dll
Binary file not shown.
Binary file not shown.
11 changes: 5 additions & 6 deletions test/TestProjectWithAnalyzers/TestProjectWithAnalyzers.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -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>