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

Add SQL code analysis from PackageReference or ProjectReference #479

Merged
merged 8 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

<!-- Microsoft.Build.Sql SDK will use DLLs from this version of DacFx Nuget. -->
<DacFxPackageVersion Condition="'$(DacFxPackageVersion)' == ''">162.3.566</DacFxPackageVersion>
<!-- TODO: update dacfx version -->
</PropertyGroup>
<ItemGroup>
<None Include="$(EnlistmentRoot)\images\nuspecicon.png" Pack="true" PackagePath="" />
Expand Down
3 changes: 3 additions & 0 deletions src/Microsoft.Build.Sql/sdk/Sdk.props
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,15 @@
<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">

<Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
<Import Project="$(MSBuildSdksPath)\Microsoft.NET.Sdk\targets\Microsoft.NET.SupportedTargetFrameworks.props" Condition="Exists('$(MSBuildSdksPath)\Microsoft.NET.Sdk\targets\Microsoft.NET.SupportedTargetFrameworks.props')" />

<PropertyGroup>
<NetCoreBuild Condition="'$(NetCoreBuild)' == '' And '$(MSBuildRuntimeType)' == 'Core'">true</NetCoreBuild>
<NetCoreBuild Condition="'$(NetCoreBuild)' == '' And '$(MSBuildRuntimeType)' == 'Full'">false</NetCoreBuild>
<NETCoreTargetsPath Condition="$(NETCoreTargetsPath) == ''">$(MSBuildThisFileDirectory)..\tools\netstandard2.1</NETCoreTargetsPath>
<TargetFramework Condition="'$(TargetFramework)' == '' AND '$(NetCoreBuild)' == 'true'">netstandard2.1</TargetFramework>
<!-- Allow packages of all target frameworks to be referenced by the sqlproj -->
<PackageTargetFallback Condition="'$(PackageTargetFallback)' == ''">@(SupportedTargetFramework->'%(Alias)')</PackageTargetFallback>
</PropertyGroup>

<!-- building in Visual Studio requires some sort of TargetFrameworkVersion. So we condition to NetCoreBuild as false to avoid failures -->
Expand Down
77 changes: 77 additions & 0 deletions test/Microsoft.Build.Sql.Tests/CodeAnalysisTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System.Collections.Generic;
using System.IO;
using NUnit.Framework;

namespace Microsoft.Build.Sql.Tests
{
[TestFixture]
public class CodeAnalysisTests : DotnetTestBase
{
[Test]
public void VerifyCodeAnalyzerFromProjectReference()
{
string tempFolder = Path.Combine(Path.GetTempPath(), TestContext.CurrentContext.Test.Name);
TestUtils.CopyDirectoryRecursive(Path.Combine(this.CommonTestDataDirectory, "CodeAnalyzerSample"), tempFolder);

// Add the analyzer csproj as a ProjectReference to the test sqlproj
ProjectUtils.AddItemGroup(this.GetProjectFilePath(), "ProjectReference",
new string[] { Path.Combine(tempFolder, "CodeAnalyzerSample.csproj") },
item =>
{
item.AddMetadata("PrivateAssets", "All");
item.AddMetadata("ReferenceOutputAssembly", "False");
item.AddMetadata("OutputItemType", "Analyzer");
item.AddMetadata("SetTargetFramework", "TargetFramework=netstandard2.1");
});

// Set up code analysis properties
ProjectUtils.AddProperties(this.GetProjectFilePath(), new Dictionary<string, string>()
{
{ "RunSqlCodeAnalysis", "true" },
{ "SqlCodeAnalysisRules", "+!CodeAnalyzerSample.TableNameRule001" } // Should fail build on this rule
});

int exitCode = this.RunDotnetCommandOnProject("build", out string stdOutput, out string stdError);

Assert.AreNotEqual(0, exitCode, "Build should have failed");
Assert.IsTrue(stdOutput.Contains("Table name [dbo].[NotAView] ends in View. This can cause confusion and should be avoided"), "Unexpected stderr");
}

[Test]
public void VerifyCodeAnalyzerFromPackageReference()
{
// Set up and create the analyzer package
string tempFolder = Path.Combine(Path.GetTempPath(), TestContext.CurrentContext.Test.Name);
TestUtils.CopyDirectoryRecursive(Path.Combine(CommonTestDataDirectory, "CodeAnalyzerSample"), tempFolder);
RunGenericDotnetCommand($"pack {Path.Combine(tempFolder, "CodeAnalyzerSample.csproj")} -o {tempFolder}", out _, out _);

// Copy analyzer package to local Nuget source
string analyzerPackagePath = Path.Combine(tempFolder, "CodeAnalyzerSample.1.0.0.nupkg");
FileAssert.Exists(analyzerPackagePath);
File.Copy(analyzerPackagePath, Path.Combine(WorkingDirectory, "pkg", "CodeAnalyzerSample.1.0.0.nupkg"));

// Add the analyzer package as a PackageReference to the test sqlproj
ProjectUtils.AddItemGroup(this.GetProjectFilePath(), "PackageReference",
new string[] { "CodeAnalyzerSample" },
item =>
{
item.AddMetadata("Version", "1.0.0");
});

// Set up code analysis properties
ProjectUtils.AddProperties(this.GetProjectFilePath(), new Dictionary<string, string>()
{
{ "RunSqlCodeAnalysis", "true" },
{ "SqlCodeAnalysisRules", "+!CodeAnalyzerSample.TableNameRule001" } // Should fail build on this rule
});

int exitCode = this.RunDotnetCommandOnProject("build", out string stdOutput, out string stdError);

Assert.AreNotEqual(0, exitCode, "Build should have failed");
Assert.IsTrue(stdOutput.Contains("Table name [dbo].[NotAView] ends in View. This can cause confusion and should be avoided"), "Unexpected stderr");
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<Project Sdk="Microsoft.NET.Sdk">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a rules author, do I really need to add all this cruft to my project to build an analyzer package? Seems complex and error prone..

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find a straightforward sample either, seems like .NET doesn't have an easy path to creating an analyzer package. The sample I used is from https://github.com/dotnet/samples/blob/main/csharp/roslyn-sdk/Tutorials/MakeConst/MakeConst.Package/MakeConst.Package.csproj

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, this is not a roslyn analyzer and DacFx has its own analysis result mechanism.

But OK - there will be few publishers of rules and hopefully many consumers

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can add this in a future dotnet new template @dzsquared

<PropertyGroup>
<OutputType>Library</OutputType>
<TargetFrameworks>netstandard2.1</TargetFrameworks>
<TargetsForTfmSpecificContentInPackage>$(TargetsForTfmSpecificContentInPackage);_AddAnalyzersToOutput</TargetsForTfmSpecificContentInPackage>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Microsoft.SqlServer.DacFx" Version="162.3.566" PrivateAssets="all" />
</ItemGroup>
<Target Name="GetTargetPath" />
<Target Name="_AddAnalyzersToOutput">
<ItemGroup>
<TfmSpecificPackageFile Include="$(OutputPath)\CodeAnalyzerSample.dll" PackagePath="analyzers/dotnet/cs" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this uncommon location? How about "tools" ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, but this is not a roslyn analyzer and the target language is not "cs" ...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I can understand that, my motivation was to keep it similar to C# and use Nuget's package resolution logic. We did build in the extensibility via SqlCodeAnalysisAssemblyPaths so if there is a growing demand for another location, we can easily hook that up.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nuget's package resolution logic

Fair, if that adds value for you.

But maybe just use analyzers\dotnet\ ??

</ItemGroup>
</Target>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
//------------------------------------------------------------------------------
// <copyright>
// Copyright (c) Microsoft Corporation. All rights reserved.
// </copyright>
//----------------------------------------------------------------------------
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using Microsoft.SqlServer.Dac.Extensibility;
using Microsoft.SqlServer.Dac.CodeAnalysis;
using Microsoft.SqlServer.Dac.Model;

namespace CodeAnalyzerSample
{

/// <summary>
/// Simple test class - note it doesn't use resources since these aren't handled by the test harness
/// that builds dll files
/// </summary>
[ExportCodeAnalysisRule("CodeAnalyzerSample.TableNameRule001",
"SampleRule",
Description = "Table names should not end in 'View'",
Category = "Naming",
PlatformCompatibility = TSqlPlatformCompatibility.OnPremises)]
class TableNameEndingInViewRule : SqlCodeAnalysisRule
{
private static readonly ModelTypeClass[] _supportedElementTypes = new[] { ModelSchema.Table };

public TableNameEndingInViewRule()
{
SupportedElementTypes = new[] { Table.TypeClass };
}

public override IList<SqlRuleProblem> Analyze(SqlRuleExecutionContext ruleExecutionContext)
{
List<SqlRuleProblem> problems = new List<SqlRuleProblem>();

TSqlObject table = ruleExecutionContext.ModelElement;
if (table != null)
{
if (NameEndsInView(table.Name))
{
string problemDescription = string.Format("Table name {0} ends in View. This can cause confusion and should be avoided",
GetQualifiedTableName(table.Name));
SqlRuleProblem problem = new SqlRuleProblem(problemDescription, table);
//SqlRuleProblem problem = new SqlRuleProblem(this, problemDescription, table);
zijchen marked this conversation as resolved.
Show resolved Hide resolved
problems.Add(problem);
}
}

return problems;
}

private bool NameEndsInView(ObjectIdentifier id)
{
return id.HasName && id.Parts.Last().EndsWith("View", StringComparison.OrdinalIgnoreCase);
}

private string GetQualifiedTableName(ObjectIdentifier id)
{
StringBuilder buf = new StringBuilder();
foreach (string part in id.Parts)
{
if (buf.Length > 0)
{
buf.Append('.');
}
buf.Append('[').Append(part).Append(']');
}
return buf.ToString();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
CREATE TABLE NotAView (
Col VARCHAR(255)
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
CREATE TABLE NotAView (
Col VARCHAR(255)
)
Loading