DacFx icon indicating copy to clipboard operation
DacFx copied to clipboard

Build ouput seems to be badly formed, causing unexpected formatting of error list

Open ErikEJ opened this issue 1 year ago • 7 comments

Build from a .sqlproj in VS 17.9.1:

C:\Code\Github\EFCorePowerTools\test\ScaffoldingTester\Database\dbo\Procedures\Procedure1.sql(1,24): Warning:  : SR0016 : Microsoft.Rules.Data : Stored procedure(sp_Procedure1) includes sp_ prefix in its name.
C:\Code\Github\EFCorePowerTools\test\ScaffoldingTester\Database\dbo\Procedures\Procedure1.sql(1,1): Warning:  : SRN0002 : SqlServer.Rules : Avoid 'sp_' prefix when naming stored procedures.
C:\Code\Github\EFCorePowerTools\test\ScaffoldingTester\Database\dbo\Procedures\Procedure1.sql(1,1): Warning:  : SRP0005 : SqlServer.Rules : SET NOCOUNT ON is recommended to be enabled in stored procedures and triggers.
C:\Code\Github\EFCorePowerTools\test\ScaffoldingTester\Database\dbo\Tables\Table1.sql(1,1): Warning:  : SRN0006 : SqlServer.Rules : Two part naming on objects is required.
C:\Code\Github\EFCorePowerTools\test\ScaffoldingTester\Database\dbo.DepartmentHistory(1,1): Warning:  : SRD0002 : SqlServer.Rules : Table does not have a primary key.
C:\Code\Github\EFCorePowerTools\test\ScaffoldingTester\Database\dbo.DepartmentHistory(12,1): Warning:  : SRN0007 : SqlServer.Rules : Index 'ix_DepartmentHistory' does not follow the company naming standard. Please use a format that starts with IX_DepartmentHistory*.

=> Error list:

image

Build from some other tool:

1>C:\Code\Github\MSBuild.Sdk.SqlProj\test\TestProjectWithAnalyzers\Procedures\sp_Test.sql(4,12): warning SR0001: Microsoft.Rules.Data : The shape of the result set produced by a SELECT * statement will change if the underlying table or view structure changes.
1>C:\Code\Github\MSBuild.Sdk.SqlProj\test\TestProjectWithAnalyzers\Procedures\sp_Test.sql(1,24): warning SR0016: Microsoft.Rules.Data : Stored procedure(sp_Test) includes sp_ prefix in its name.
1>C:\Code\Github\MSBuild.Sdk.SqlProj\test\TestProjectWithAnalyzers\Procedures\sp_Test.sql(1,1): warning SRN0002: SqlServer.Rules : Avoid 'sp_' prefix when naming stored procedures.
1>C:\Code\Github\MSBuild.Sdk.SqlProj\test\TestProjectWithAnalyzers\Procedures\sp_Test.sql(1,1): warning SRP0005: SqlServer.Rules : SET NOCOUNT ON is recommended to be enabled in stored procedures and triggers.

=> Better error list (IMHO)

Seems to be cause by the multiple : signs here Warning: : SR0016 :

Docs

ErikEJ avatar Feb 28 '24 11:02 ErikEJ

this might be a bug in SSDT instead of dacfx

llali avatar Feb 29 '24 17:02 llali

@llali agree, it is related to how SSDT reports build results, not DacFX as such

ErikEJ avatar Feb 29 '24 18:02 ErikEJ

While the bug surfaces in SSDT, it seems like its DacFx outputing build errors with the extra semicolon (between Warning: SR0016) on code analysis rules validation. That's a DacFx item I would suggest we keep for future improvement.

dzsquared avatar May 13 '24 15:05 dzsquared

Looking forward.. Now, if only DacFX was open source 😅

ErikEJ avatar May 13 '24 15:05 ErikEJ

I have a fix for this in DacFx. Here's the output from dotnet build generated from my test after the fix: "StaticCodeAnalysis warning SR0001: Microsoft.Rules.Data : The shape of the result set produced by a SELECT * statement will change if the underlying table or view structure changes.", "StaticCodeAnalysis warning SR0016: Microsoft.Rules.Data : Stored procedure(sp_Procedure1) includes sp_ prefix in its name.", "warning SQL71502: Procedure: [dbo].[sp_Procedure1] has an unresolved reference to object [dbo].[InvalidTable]"

llali avatar May 14 '24 17:05 llali

@llali Great news. I am surprised that DacFX actually generates the analysis output and nit SSDT. What is the timeline for the fix? VS 17.11??

ErikEJ avatar May 14 '24 19:05 ErikEJ

@ErikEJ - yes, this change would first appear in a preview of a future (17.11) release of VS. I can't venture which preview just yet. :) As I'm sure you guessed, it will not appear in 17.10.

kudos @llali for pinning the message anomaly down in dacfx

dzsquared avatar May 14 '24 19:05 dzsquared