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

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

Closed
ErikEJ opened this issue Feb 28, 2024 · 15 comments
Assignees
Labels
area: ssdt Related to the use of DacFx in Visual Studio (SSDT) bug Something isn't working fixed-pending-release Fix in upcoming release
Milestone

Comments

@ErikEJ
Copy link
Contributor

ErikEJ commented Feb 28, 2024

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 ErikEJ added the bug Something isn't working label Feb 28, 2024
@dzsquared dzsquared added the area: ssdt Related to the use of DacFx in Visual Studio (SSDT) label Feb 28, 2024
@llali
Copy link
Member

llali commented Feb 29, 2024

this might be a bug in SSDT instead of dacfx

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Feb 29, 2024

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

@dzsquared
Copy link
Contributor

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
Copy link
Contributor Author

ErikEJ commented May 13, 2024

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

@llali llali self-assigned this May 14, 2024
@llali
Copy link
Member

llali commented May 14, 2024

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
Copy link
Contributor Author

ErikEJ commented May 14, 2024

@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
Copy link
Contributor

@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
Copy link
Contributor

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.

@dzsquared dzsquared added the fixed-pending-release Fix in upcoming release label May 28, 2024
@dzsquared dzsquared added this to the 162.3 milestone May 28, 2024
@llali
Copy link
Member

llali commented Jun 6, 2024

fixed in 162.3

@llali llali closed this as completed Jun 6, 2024
@ErikEJ
Copy link
Contributor Author

ErikEJ commented Aug 14, 2024

@dzsquared Was this supposed to be fixed in 17.11 - I do not see it !?

@dzsquared
Copy link
Contributor

argh - 17.11 shipped with DacFx 162.2.127

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Aug 14, 2024

Nooooo.. why is it so far behind??

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Aug 14, 2024

So what VS version will have 162.3??

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Sep 21, 2024

VS 17.12 preview 2 - still broken @dzsquared @llali - is that expected?

image

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Sep 21, 2024

At least it works with .sqlprojx:

1>C:\Users..\source\repos\Database2\Database2\Table1.sql(1,1,1,1): StaticCodeAnalysis warning SRN0006: SqlServer.Rules : Two part naming on objects is required.
1>C:\Users..\source\repos\Database2\Database2\Table1.sql(1,1,1,1): StaticCodeAnalysis warning SRN0007: SqlServer.Rules : PrimaryKeyConstraint found without a name.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ssdt Related to the use of DacFx in Visual Studio (SSDT) bug Something isn't working fixed-pending-release Fix in upcoming release
Projects
None yet
Development

No branches or pull requests

4 participants