Skip to content

Commit

Permalink
Enable .NET Analyzers in DacpacTool
Browse files Browse the repository at this point in the history
fixes #602
  • Loading branch information
ErikEJ committed Aug 5, 2024
1 parent f9fb9f2 commit 1d01eec
Show file tree
Hide file tree
Showing 11 changed files with 83 additions and 16 deletions.
3 changes: 3 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,6 @@ csharp_preserve_single_line_blocks = true
[*.vb]
# Modifier preferences
visual_basic_preferred_modifier_order = Partial,Default,Private,Protected,Public,Friend,NotOverridable,Overridable,MustOverride,Overloads,Overrides,MustInherit,NotInheritable,Static,Shared,Shadows,ReadOnly,WriteOnly,Dim,Const,WithEvents,Widening,Narrowing,Custom,Async:suggestion

[**/Microsoft.SqlTools.ManagedBatchParser/**.cs]
generated_code = true
2 changes: 1 addition & 1 deletion src/DacpacTool/BuildOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

namespace MSBuild.Sdk.SqlProj.DacpacTool
{
public class BuildOptions : BaseOptions
internal class BuildOptions : BaseOptions

Check warning on line 6 in src/DacpacTool/BuildOptions.cs

View workflow job for this annotation

GitHub Actions / build

'BuildOptions' is an internal class that is apparently never instantiated. If so, remove the code from the assembly. If this class is intended to contain only static members, make it 'static' (Module in Visual Basic). (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1812)

Check warning on line 6 in src/DacpacTool/BuildOptions.cs

View workflow job for this annotation

GitHub Actions / build

Type 'BuildOptions' can be sealed because it has no subtypes in its containing assembly and is not externally visible (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1852)

Check warning on line 6 in src/DacpacTool/BuildOptions.cs

View workflow job for this annotation

GitHub Actions / build

'BuildOptions' is an internal class that is apparently never instantiated. If so, remove the code from the assembly. If this class is intended to contain only static members, make it 'static' (Module in Visual Basic). (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1812)
{
public string Name { get; set; }
public string Version { get; set; }
Expand Down
14 changes: 9 additions & 5 deletions src/DacpacTool/DacpacTool.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
<OutputType>Exe</OutputType>
<TargetFrameworks>net6.0;net8.0</TargetFrameworks>
<LangVersion>9.0</LangVersion>
<AnalysisMode>all</AnalysisMode>
<EnableNETAnalyzers>true</EnableNETAnalyzers>
<AnalysisLevel>latest</AnalysisLevel>
<EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
</PropertyGroup>

<PropertyGroup>
Expand All @@ -25,11 +29,11 @@
<ItemGroup>
<PackageReference Include="Microsoft.SqlServer.SqlManagementObjects" Version="171.30.0" />
<ProjectReference Include="$(SqlToolsPath)/Microsoft.SqlTools.Hosting/Microsoft.SqlTools.Hosting.csproj" />
<Compile Include="$(ManagedBatchParserPath)/Localization/*.cs" />
<EmbeddedResource Include="$(ManagedBatchParserPath)/Localization/sr.resx" LogicalName="Microsoft.SqlTools.ManagedBatchParser.Localization.SR.resources" />
<None Include="$(ManagedBatchParserPath)/Localization/sr.strings" />
<Compile Include="$(ManagedBatchParserPath)/BatchParser/**/*.cs" />
<Compile Include="$(ManagedBatchParserPath)/ReliableConnection/**/*.cs" />
<Compile Include="$(ManagedBatchParserPath)/Localization/*.cs" Link="BatchParser/%(Filename)%(Extension)" />
<EmbeddedResource Include="$(ManagedBatchParserPath)/Localization/sr.resx" LogicalName="Microsoft.SqlTools.ManagedBatchParser.Localization.SR.resources" Link="BatchParser/%(Filename)%(Extension)" />
<None Include="$(ManagedBatchParserPath)/Localization/sr.strings" Link="BatchParser/%(Filename)%(Extension)" />
<Compile Include="$(ManagedBatchParserPath)/BatchParser/**/*.cs" Link="BatchParser/%(Filename)%(Extension)" />
<Compile Include="$(ManagedBatchParserPath)/ReliableConnection/**/*.cs" Link="BatchParser/%(Filename)%(Extension)" />
</ItemGroup>

</Project>
2 changes: 1 addition & 1 deletion src/DacpacTool/DeployOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

namespace MSBuild.Sdk.SqlProj.DacpacTool
{
public class DeployOptions : BaseOptions
internal class DeployOptions : BaseOptions

Check warning on line 5 in src/DacpacTool/DeployOptions.cs

View workflow job for this annotation

GitHub Actions / build

'DeployOptions' is an internal class that is apparently never instantiated. If so, remove the code from the assembly. If this class is intended to contain only static members, make it 'static' (Module in Visual Basic). (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1812)

Check warning on line 5 in src/DacpacTool/DeployOptions.cs

View workflow job for this annotation

GitHub Actions / build

Type 'DeployOptions' can be sealed because it has no subtypes in its containing assembly and is not externally visible (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1852)

Check warning on line 5 in src/DacpacTool/DeployOptions.cs

View workflow job for this annotation

GitHub Actions / build

'DeployOptions' is an internal class that is apparently never instantiated. If so, remove the code from the assembly. If this class is intended to contain only static members, make it 'static' (Module in Visual Basic). (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1812)
{
public FileInfo Input { get; set; }
public string TargetServerName { get; set; }
Expand Down
29 changes: 24 additions & 5 deletions src/DacpacTool/Extensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ public static class Extensions
{
public static string Format(this BatchErrorEventArgs args, string source)
{
ArgumentNullException.ThrowIfNull(args);

var outputMessageBuilder = new StringBuilder();
outputMessageBuilder.Append(source);
outputMessageBuilder.Append('(');
Expand All @@ -42,6 +44,8 @@ public static string Format(this BatchErrorEventArgs args, string source)

public static string Format(this BatchParserExecutionErrorEventArgs args, string source)
{
ArgumentNullException.ThrowIfNull(args);

var outputMessageBuilder = new StringBuilder();
outputMessageBuilder.Append(source);
outputMessageBuilder.Append('(');
Expand Down Expand Up @@ -74,15 +78,17 @@ public static string Format(this BatchParserExecutionErrorEventArgs args, string

public static string Format(this DacModelException exception, string fileName)
{
ArgumentNullException.ThrowIfNull(exception);

var stringBuilder = new StringBuilder();

foreach (var modelError in exception.Messages)
{
stringBuilder.Append(fileName);
stringBuilder.Append('(');
stringBuilder.Append("1");
stringBuilder.Append('1');
stringBuilder.Append(',');
stringBuilder.Append("1");
stringBuilder.Append('1');
stringBuilder.Append("):");
stringBuilder.Append(' ');
stringBuilder.Append("Error");
Expand All @@ -98,6 +104,8 @@ public static string Format(this DacModelException exception, string fileName)

public static string GetPreDeploymentScript(this DacPackage package)
{
ArgumentNullException.ThrowIfNull(package);

var stream = package.PreDeploymentScript;
if (stream == null)
{
Expand All @@ -110,6 +118,8 @@ public static string GetPreDeploymentScript(this DacPackage package)

public static string GetPostDeploymentScript(this DacPackage package)
{
ArgumentNullException.ThrowIfNull(package);

var stream = package.PostDeploymentScript;
if (stream == null)
{
Expand All @@ -122,6 +132,8 @@ public static string GetPostDeploymentScript(this DacPackage package)

public static void AddReference(this TSqlModel model, string referencePath, string externalParts, bool suppressErrorsForMissingDependencies)
{
ArgumentNullException.ThrowIfNull(model);

var dataSchemaModel = GetDataSchemaModel(model);

var customData = Activator.CreateInstance(Type.GetType("Microsoft.Data.Tools.Schema.SchemaModel.CustomSchemaData, Microsoft.Data.Tools.Schema.Sql"), "Reference", "SqlSchema");
Expand Down Expand Up @@ -150,7 +162,7 @@ private static string ParseExternalParts(string externalParts)
string databaseVariableLiteralValue = null;

// If there are '=' sign in argument assumes that this is formula, else assume that a single value passed and that it is database literal.
if (externalParts.Contains('='))
if (externalParts.Contains('=', StringComparison.OrdinalIgnoreCase))
{
foreach (Match match in new Regex(@"dbl=(?<dbl>\w+)|dbv=(?<dbv>\w+)|srv=(?<srv>\w+)",
RegexOptions.CultureInvariant, TimeSpan.FromSeconds(1)).Matches(externalParts))
Expand Down Expand Up @@ -187,7 +199,7 @@ private static string ParseExternalParts(string externalParts)
/// <summary>
/// Cached method info for FileUtils.EnsureIsDelimited
/// </summary>
private static MethodInfo _ensureIsDelimitedMethod = null;
private static MethodInfo _ensureIsDelimitedMethod;
/// <summary>
/// This method found in Microsoft.Data.Tools.Utilities in class FileUtils. because of it is internal we do call through Reflection
/// </summary>
Expand All @@ -204,6 +216,8 @@ private static string EnsureIsDelimited(string name)

public static IEnumerable<string> GetReferencedDacPackages(this TSqlModel model)
{
ArgumentNullException.ThrowIfNull(model);

var result = new List<string>();
var dataSchemaModel = GetDataSchemaModel(model);

Expand All @@ -230,6 +244,9 @@ public static IEnumerable<string> GetReferencedDacPackages(this TSqlModel model)

public static void AddSqlCmdVariables(this TSqlModel model, string[] variables)
{
ArgumentNullException.ThrowIfNull(model);
ArgumentNullException.ThrowIfNull(variables);

var dataSchemaModel = GetDataSchemaModel(model);

var customData = Activator.CreateInstance(Type.GetType("Microsoft.Data.Tools.Schema.SchemaModel.CustomSchemaData, Microsoft.Data.Tools.Schema.Sql"), "SqlCmdVariables", "SqlCmdVariable");
Expand All @@ -240,7 +257,7 @@ public static void AddSqlCmdVariables(this TSqlModel model, string[] variables)
var variableName = varWithValue[0];
string variableDefaultValue = string.Empty;

if (varWithValue.Length > 1 && varWithValue[1] != string.Empty)
if (varWithValue.Length > 1 && !string.IsNullOrEmpty(varWithValue[1]))
{
variableDefaultValue = varWithValue[1];
Console.WriteLine($"Adding SqlCmd variable {variableName} with default value {variableDefaultValue}");
Expand All @@ -257,6 +274,8 @@ public static void AddSqlCmdVariables(this TSqlModel model, string[] variables)

public static IEnumerable<ModelValidationError> GetModelValidationErrors(this TSqlModel model, IEnumerable<string> ignoreValidationErrrors)
{
ArgumentNullException.ThrowIfNull(model);

var service = model.GetType().GetField("_service", BindingFlags.NonPublic | BindingFlags.Instance).GetValue(model);
var getModelValidationErrorsMethod = service.GetType().GetMethod("GetModelValidationErrors", BindingFlags.NonPublic | BindingFlags.Instance);
var modelValidationErrors = getModelValidationErrorsMethod.Invoke(service, new object[] { ignoreValidationErrrors }) as IEnumerable<object>;
Expand Down
7 changes: 7 additions & 0 deletions src/DacpacTool/PackageAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ public PackageAnalyzer(IConsole console, string rulesExpression)

public void AddRulesFile(FileInfo inputFile)
{
ArgumentNullException.ThrowIfNull(inputFile);

// Make sure the file exists
if (!inputFile.Exists)
{
Expand All @@ -39,6 +41,9 @@ public void AddRulesFile(FileInfo inputFile)

public void Analyze(TSqlModel model, FileInfo outputFile)
{
ArgumentNullException.ThrowIfNull(model);
ArgumentNullException.ThrowIfNull(outputFile);

_console.WriteLine($"Analyzing package '{outputFile.FullName}'");
try
{
Expand Down Expand Up @@ -73,10 +78,12 @@ public void Analyze(TSqlModel model, FileInfo outputFile)
}
_console.WriteLine($"Successfully analyzed package '{outputFile}'");
}
#pragma warning disable CA1031 // Do not catch general exception types
catch (Exception ex)
{
_console.WriteLine($"ERROR: An unknown error occurred while analyzing package '{outputFile.FullName}': {ex}");
}
#pragma warning restore CA1031 // Do not catch general exception types
}

private void BuildRuleLists(string rulesExpression)
Expand Down
16 changes: 15 additions & 1 deletion src/DacpacTool/PackageBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ public void AddSqlCmdVariables(string[] variables)

public bool AddInputFile(FileInfo inputFile)
{
ArgumentNullException.ThrowIfNull(inputFile);

// Ensure that the model has been created
EnsureModelCreated();

Expand Down Expand Up @@ -100,11 +102,15 @@ public bool AddInputFile(FileInfo inputFile)

public void AddPreDeploymentScript(FileInfo script, FileInfo outputFile)
{
ArgumentNullException.ThrowIfNull(outputFile);

AddScript(script, outputFile, "/predeploy.sql");
}

public void AddPostDeploymentScript(FileInfo script, FileInfo outputFile)
{
ArgumentNullException.ThrowIfNull(outputFile);

AddScript(script, outputFile, "/postdeploy.sql");
}

Expand Down Expand Up @@ -166,6 +172,8 @@ void ProcessWarning(ModelValidationError modelError)

public void SaveToDisk(FileInfo outputFile, PackageOptions packageOptions = null)
{
ArgumentNullException.ThrowIfNull(outputFile);

// Ensure that the model has been created and metadata has been set
EnsureModelCreated();
EnsureModelValidated();
Expand Down Expand Up @@ -359,13 +367,17 @@ private static void WritePart(FileInfo file, Package package, string path)

public bool TreatTSqlWarningsAsErrors { get; set; }

private static readonly char[] separator = new [] { ',', ';' };

public void AddWarningsToSuppress(string suppressionList)
{
_suppressedWarnings.AddRange(ParseSuppressionList(suppressionList));
}

public void AddFileWarningsToSuppress(FileInfo inputFile, string suppressionList)
{
ArgumentNullException.ThrowIfNull(inputFile);

var warningList = ParseSuppressionList(suppressionList);
if (warningList.Count > 0)
{
Expand All @@ -386,7 +398,7 @@ private static List<int> ParseSuppressionList(string suppressionList)
var result = new List<int>();
if (!string.IsNullOrEmpty(suppressionList))
{
foreach (var str in suppressionList.Split(new [] { ',', ';' }, StringSplitOptions.RemoveEmptyEntries))
foreach (var str in suppressionList.Split(separator, StringSplitOptions.RemoveEmptyEntries))
{
if (int.TryParse(str.Trim(), out var value))
{
Expand All @@ -400,6 +412,8 @@ private static List<int> ParseSuppressionList(string suppressionList)

public void GenerateCreateScript(FileInfo dacpacFile, string databaseName, DacDeployOptions deployOptions)
{
ArgumentNullException.ThrowIfNull(dacpacFile);

if (string.IsNullOrWhiteSpace(databaseName))
{
throw new ArgumentException("The database name is mandatory.", nameof(databaseName));
Expand Down
8 changes: 8 additions & 0 deletions src/DacpacTool/PackageDeployer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,15 @@ public void SetSqlCmdVariable(string key, string value)

public void RunPreDeploymentScriptFromReferences(FileInfo dacpacPackage, string targetDatabaseName)
{
ArgumentNullException.ThrowIfNull(dacpacPackage);

RunDeploymentScriptFromReferences(dacpacPackage, targetDatabaseName, true);
}

public void Deploy(FileInfo dacpacPackage, string targetDatabaseName)
{
ArgumentNullException.ThrowIfNull(dacpacPackage);

EnsureConnectionStringComplete();

if (!dacpacPackage.Exists)
Expand Down Expand Up @@ -111,14 +115,18 @@ public void Deploy(FileInfo dacpacPackage, string targetDatabaseName)
_console.WriteLine($"ERROR: Deployment of database '{targetDatabaseName}' failed: {ex.Message}");
}
}
#pragma warning disable CA1031 // Do not catch general exception types
catch (Exception ex)
{
_console.WriteLine($"ERROR: An unknown error occurred while deploying database '{targetDatabaseName}': {ex.Message}");
}
#pragma warning restore CA1031 // Do not catch general exception types
}

public void RunPostDeploymentScriptFromReferences(FileInfo dacpacPackage, string targetDatabaseName)
{
ArgumentNullException.ThrowIfNull(dacpacPackage);

RunDeploymentScriptFromReferences(dacpacPackage, targetDatabaseName, false);
}

Expand Down
11 changes: 9 additions & 2 deletions src/DacpacTool/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ namespace MSBuild.Sdk.SqlProj.DacpacTool
{
class Program

Check warning on line 14 in src/DacpacTool/Program.cs

View workflow job for this annotation

GitHub Actions / build

Type 'Program' can be sealed because it has no subtypes in its containing assembly and is not externally visible (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1852)
{
[System.Diagnostics.CodeAnalysis.SuppressMessage("Performance", "CA1861:Avoid constant arrays as arguments", Justification = "Not called repeatedly")]
static async Task<int> Main(string[] args)
{
var buildCommand = new Command("build")
Expand Down Expand Up @@ -77,7 +78,9 @@ static async Task<int> Main(string[] args)
rootCommand.Description = "Command line tool for generating a SQL Server Data-Tier Application Framework package (dacpac)";

var processed = rootCommand.Parse(args);
#pragma warning disable CA2007 // Consider calling ConfigureAwait on the awaited task
return await processed.InvokeAsync();
#pragma warning restore CA2007 // Consider calling ConfigureAwait on the awaited task
}

private static int BuildDacpac(BuildOptions options)
Expand Down Expand Up @@ -301,24 +304,28 @@ private static int DeployDacpac(DeployOptions options)
Console.WriteLine($"ERROR: An error occured while validating arguments: {ex.Message}");
return 1;
}
#pragma warning disable CA1031 // Do not catch general exception types
catch (Exception ex)
{
Console.WriteLine($"ERROR: An error ocurred during deployment: {ex.Message}");
return 1;
}
#pragma warning restore CA1031 // Do not catch general exception types
}

[Conditional("DEBUG")]
private static void WaitForDebuggerToAttach(BaseOptions options)
{
if (options.Debug)
{
Console.WriteLine($"Waiting for debugger to attach ({System.Diagnostics.Process.GetCurrentProcess().Id})");
Console.WriteLine($"Waiting for debugger to attach ({Environment.ProcessId})");
while (!Debugger.IsAttached)
{
Thread.Sleep(100);
}
Console.WriteLine("Debugger attached");
#pragma warning disable CA1303 // Do not pass literals as localized parameters
Console.WriteLine(@"Debugger attached");
#pragma warning restore CA1303 // Do not pass literals as localized parameters
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/DacpacTool/PropertyParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ namespace MSBuild.Sdk.SqlProj.DacpacTool
/// <summary>
/// Allows to parse properties from build or deploy options
/// </summary>
public static class PropertyParser
internal static class PropertyParser
{
private static readonly Dictionary<string, Func<string, object>> CustomParsers = new Dictionary<string, Func<string, object>>();

Expand Down
5 changes: 5 additions & 0 deletions src/DacpacTool/ScriptInspector.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.IO;
using System.Collections.Generic;
using System;

namespace MSBuild.Sdk.SqlProj.DacpacTool
{
Expand All @@ -13,11 +14,15 @@ public IEnumerable<string> IncludedFiles

public void AddPreDeploymentScript(FileInfo script)
{
ArgumentNullException.ThrowIfNull(script);

AddIncludedFiles(script);
}

public void AddPostDeploymentScript(FileInfo script)
{
ArgumentNullException.ThrowIfNull(script);

AddIncludedFiles(script);
}

Expand Down

0 comments on commit 1d01eec

Please sign in to comment.