microsoft / DacFx

SQL Server database schema validation, deployment, and upgrade runtime. Enables declarative database development and database portability across SQL Server versions and environments.
https://aka.ms/sqlpackage-ref
MIT License
292 stars 15 forks source link

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

Closed ErikEJ closed 3 weeks ago

ErikEJ commented 4 months ago

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 hereWarning: : SR0016 :

Docs

llali commented 4 months ago

this might be a bug in SSDT instead of dacfx

ErikEJ commented 4 months ago

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

dzsquared commented 1 month ago

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.

ErikEJ commented 1 month ago

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

llali commented 1 month ago

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]"

ErikEJ commented 1 month ago

@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??

dzsquared commented 1 month ago

@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 commented 1 month ago

This landed in https://www.nuget.org/packages/Microsoft.SqlServer.DacFx/162.3.557-preview, not positive which 17.11 preview this will hit yet.

llali commented 3 weeks ago

fixed in 162.3