MSBuild.Sdk.SqlProj
MSBuild.Sdk.SqlProj copied to clipboard
rr-wfm
Question: Compatibility with SSDT rules?
I wrote some rules that we use during our build process for SSDT. I was wondering if you or anyone else have tested out if the rules still work with your projects. I am very interested in being able to use nuget with SSDT.
The rules I wrote are here, if you are interested: https://github.com/tcartwright/SqlServer.Rules
Sorry for using the issues to submit you a question.
Hi @tcartwright. First of all thank you for asking this question. I don't have a lot of experience with custom rules so I haven't tried this myself yet. That being said I can't really think of a reason why it can't work, it is more a matter of figuring out how to make it work. Looking at your README it seems that just placing the assemblies in the right location would trigger them at build them for regular SQL Database Projects. Is that correct? Or do you need to put something into the project file in any way?
Hey @jmezach, thank you very much for the response. I appreciate it very much. Getting the rules in place is actually a two step process. Really should update the readme. :) Once you place the assemblies in place you turn on custom code analysis in the project properties. You will also need to select any rules that are unselected in the list or deselect any you don't want to run. If you apply my custom rules in the correct directory beforehand, they should also show up in the list of rules to run. Once code analysis is enabled then just perform a rebuild. Let me know if you want / need any help with it.
I would love to get your project in use at my current company. Like you I did a lot of work with SSDT. Our stories sound very similar. I built a deployment system around TFS build / Octopus Deploy wrapped around Red Gate SQL Compare. I also integrated t-sqlt unit tests into our build with code coverage. I was highly interested when I read your blog post and what you had done.
NOTE: Once you run a build with code analysis enabled VS will place a hard lock on the rule assemblies. To update them or remove them you will need to shut down any SSDT projects Visual Studios.
@tcartwright Can you share the changes made to the project file after you've enabled your custom rules? That might give some idea for how we might go about enabling this in MSBuild.Sdk.SqlProj.
As for your NOTE, if we're able to get this working I'm guessing that hard lock goes away since the build phase happens outside of Visual Studio with MSBuild.Sdk.SqlProj. Unfortunately that also means that you only get the warnings on build and not while editing your code.
@jmezach here are the changes made to a normal SSDT project to enable the analysis:
Turned on, all rules enabled:
<RunSqlCodeAnalysis>True</RunSqlCodeAnalysis>
Enabled, some rules disabled:
<RunSqlCodeAnalysis>True</RunSqlCodeAnalysis> <SqlCodeAnalysisRules>-Microsoft.Rules.Data.SR0011;-Microsoft.Rules.Data.SR0012;-Microsoft.Rules.Data.SR0016</SqlCodeAnalysisRules>
Enabled, some rules marked as errors:
<RunSqlCodeAnalysis>True</RunSqlCodeAnalysis> <SqlCodeAnalysisRules>+!Microsoft.Rules.Data.SR0001;+!Microsoft.Rules.Data.SR0008</SqlCodeAnalysisRules>
Sadly, the code analysis for this is never live while editing code, it only ever happens on build. It was the way Microsoft designed it. Think more FxCop and not Roslyn. Although that would be a good thing to write.... Roslyn analysis rules for sql files. Hmmm....
There are no actual changes to the project file once we place our rules on a machine. The project file does not actually change. The rules that are on that machine just show up, and run if code analysis is enabled, and the rules are not explicitly disabled.
On our server build, I actually override the RunSqlCodeAnalysis for our MSBuild and enable it. We eventually also have plans to force some rules as errors. That way we are not dependent upon how the developer configured his project.
@tcartwright @jmezach I have done some investigation of this in connection with another project I am considering, and have forked SqlServer.Rules, updated to NET 8 and latest DacFX, and I am able to run the SqlServer.Rules.Generator command line tool and generate a XML and CSV output from the rules analysis.
The current tool takes a TSqlModel/.dacpac as parameter, together with a suppression list.
With a little refactoring I think this could be incorporated and included in the DacpacTool.
Initially we could just support the <RunSqlCodeAnalysis>True</RunSqlCodeAnalysis> setting, and then add support for the formats used in <SqlCodeAnalysisRules> later.
Challenge could be to get the rules DLL to be present in the tools folder, but we could just include it initially.
@tcartwright Interested in a PR that updates the SqlServer.Rules and SqlServer.Rules.Report libraries to also target net6/net8 ?
It could be cool to be able to consume a rules .dll as a nuget package, and get them copied to the dacpactool folder during build.
Do we need it to be in the DacpacTool folder? Or can we just point at it somehow?
Initially, we could simply include the two additional useful rules sets that I am aware of as a dacpactool package reference?
(Once NuGet package(s) with these are available)
https://github.com/tcartwright/SqlServer.Rules https://github.com/davebally/TSQL-Smells
This is the (pseudo) code required to run analysis:
var factory = new CodeAnalysisServiceFactory();
var service = factory.CreateAnalysisService(packageBuilder.Model);
var result = service.Analyze(model);
if (!result.AnalysisSucceeded)
{
foreach (var err in result.InitializationErrors)
{
SendNotification(err.Message, NotificationType.Error);
}
foreach (var err in result.SuppressionErrors)
{
SendNotification(err.Message, NotificationType.Error);
}
foreach (var err in result.AnalysisErrors)
{
SendNotification(err.Message, NotificationType.Error);
}
return;
}
else
{
foreach (var err in result.Problems)
{
SendNotification(err.ErrorMessageString, NotificationType.Warning);
}
result.SerializeResultsToXml(GetOutputFileName(options.Output));
}
Looks easy enough. I'm wondering whether this should be a separate command for the DacpacTool or whether it would be part of the existing build command.
.sqlproj supports both types, but I think initially it makes sense to add it to the existing build command, of course only run if <RunSqlCodeAnalysis>True</RunSqlCodeAnalysis>
@jmezach I am looking at src\DacpacTool\ModelValidationError.cs - this formatting is "magic" so you can navigate to the source file, is that the intention of the formatting?
Yes, that formatting is intentional. I believe that makes VS pick up these messages and present them in the Error And Warnings window.
Thoughts on "bring your own rules":
- publish your own rules as NuGet package that targets netstandard2.0
- Enhance SDK to include rules .dll from /lib/netstandard2.0 virtual folder i packagereferenced package
- If codeanalysis is enabled, add an addtional command to dacpactool to load rules DLL files from output folder (folder name supplied as parameter) - rules .dll files could be required to have special naming (*.Rules.dll)
As an alternative to the above, a user can place the rules .dll manually in the C:\Users\<user>\.nuget\packages\msbuild.sdk.sqlproj\2.6.1\tools\net8.0 folder
This is not so different from the standard approach
@jmezach @jeffrosenberg I investigated the effect of TreatwarningsAsErrors - it has no effect on .sqlproj - the only way to make a analyzer problem an error is to mark it explicitly:
<RunSqlCodeAnalysis>True</RunSqlCodeAnalysis>
<SqlCodeAnalysisRules>+!Microsoft.Rules.Data.SR0001;+!Microsoft.Rules.Data.SR0008</SqlCodeAnalysisRules>