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

Analyzers #515

Merged
merged 11 commits into from
Feb 26, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/DacpacTool/BuildOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down
2 changes: 2 additions & 0 deletions src/DacpacTool/DacpacTool.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="ErikEJ.DacFX.SqlServer.Rules" Version="1.0.0-preview3" />
<PackageReference Include="ErikEJ.DacFX.TSQLSmellSCA" Version="1.0.0-preview1" />
<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" />
Expand Down
33 changes: 33 additions & 0 deletions src/DacpacTool/ExtensibilityErrorExtensions.cs
Original file line number Diff line number Diff line change
@@ -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();
}
}
}
97 changes: 97 additions & 0 deletions src/DacpacTool/PackageAnalyzer.cs
Original file line number Diff line number Diff line change
@@ -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";
}
}
}
9 changes: 9 additions & 0 deletions src/DacpacTool/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down Expand Up @@ -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;
}

Expand Down
31 changes: 31 additions & 0 deletions src/DacpacTool/SqlRuleProblemExtensions.cs
Original file line number Diff line number Diff line change
@@ -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();
}
}
}
4 changes: 3 additions & 1 deletion src/MSBuild.Sdk.SqlProj/Sdk/Sdk.targets
Original file line number Diff line number Diff line change
Expand Up @@ -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)" />
Expand Down
87 changes: 87 additions & 0 deletions test/DacpacTool.Tests/PackageAnalyzerTests.cs
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
20 changes: 20 additions & 0 deletions test/DacpacTool.Tests/TestConsole.cs
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
11 changes: 9 additions & 2 deletions test/DacpacTool.Tests/TestModelBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
5 changes: 5 additions & 0 deletions test/TestProjectWithAnalyzers/Procedures/sp_Test.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
CREATE PROCEDURE [dbo].[sp_Test]
AS
BEGIN
SELECT * FROM [dbo].[MyTable];
END
12 changes: 12 additions & 0 deletions test/TestProjectWithAnalyzers/TestProjectWithAnalyzers.csproj
Original file line number Diff line number Diff line change
@@ -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>