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.

Sign up for GitHub

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

Jump to bottom

Compile successfully - without IDE0055 - from a fresh checkout #31993

Merged
merged 1 commit into from
Dec 28, 2018
Merged

Compile successfully - without IDE0055 - from a fresh checkout #31993

merged 1 commit into from
Dec 28, 2018

Conversation

mikkelbu
Copy link
Contributor

When I run Restore.cmd and then Build.cmd in a fresh clone I get the following formatting errors (all IDE0055):

Build FAILED.

C:\src\DotNet\roslyn\src\Compilers\CSharp\Portable\SymbolDisplay\SymbolDisplayVisitor.Members.cs(52,1): error IDE0055: Fix formatting [C:\src\DotNet\roslyn\src\Compilers\CSharp\Portable\Microsoft.CodeAnalysis.CSharp.csproj]
C:\src\DotNet\roslyn\src\Compilers\VisualBasic\Test\IOperation\IOperation\IOperationTests_IUnaryOperatorExpression.vb(2211,1): error IDE0055: Fix formatting [C:\src\DotNet\roslyn\src\Compilers\VisualBasic\Test\IOperation\Roslyn.Compilers.VisualBasic.IOperation.UnitTests.vbproj]
C:\src\DotNet\roslyn\src\Compilers\VisualBasic\Test\IOperation\IOperation\IOperationTests_IUnaryOperatorExpression.vb(2251,1): error IDE0055: Fix formatting [C:\src\DotNet\roslyn\src\Compilers\VisualBasic\Test\IOperation\Roslyn.Compilers.VisualBasic.IOperation.UnitTests.vbproj]
C:\src\DotNet\roslyn\src\Workspaces\CSharp\Portable\Classification\SyntaxClassification\NameSyntaxClassifier.cs(138,62): error IDE0055: Fix formatting [C:\src\DotNet\roslyn\src\Workspaces\CSharp\Portable\Microsoft.CodeAnalysis.CSharp.Workspaces.csproj]
C:\src\DotNet\roslyn\src\Compilers\VisualBasic\Test\IOperation\IOperation\IOperationTests_IUnaryOperatorExpression.vb(2211,1): error IDE0055: Fix formatting [C:\src\DotNet\roslyn\src\Compilers\VisualBasic\Test\IOperation\Roslyn.Compilers.VisualBasic.IOperation.UnitTests.vbproj]
C:\src\DotNet\roslyn\src\Compilers\VisualBasic\Test\IOperation\IOperation\IOperationTests_IUnaryOperatorExpression.vb(2251,1): error IDE0055: Fix formatting [C:\src\DotNet\roslyn\src\Compilers\VisualBasic\Test\IOperation\Roslyn.Compilers.VisualBasic.IOperation.UnitTests.vbproj]
C:\src\DotNet\roslyn\src\EditorFeatures\CSharpTest\Classification\SemanticClassifierTests.cs(529,37): error IDE0055: Fix formatting [C:\src\DotNet\roslyn\src\EditorFeatures\CSharpTest\Microsoft.CodeAnalysis.CSharp.EditorFeatures.UnitTests.csproj]
C:\src\DotNet\roslyn\src\EditorFeatures\CSharpTest\Classification\SemanticClassifierTests.cs(543,37): error IDE0055: Fix formatting [C:\src\DotNet\roslyn\src\EditorFeatures\CSharpTest\Microsoft.CodeAnalysis.CSharp.EditorFeatures.UnitTests.csproj]
C:\src\DotNet\roslyn\src\EditorFeatures\CSharpTest\Classification\SemanticClassifierTests.cs(558,37): error IDE0055: Fix formatting [C:\src\DotNet\roslyn\src\EditorFeatures\CSharpTest\Microsoft.CodeAnalysis.CSharp.EditorFeatures.UnitTests.csproj]
C:\src\DotNet\roslyn\src\EditorFeatures\CSharpTest\Classification\SemanticClassifierTests.cs(581,37): error IDE0055: Fix formatting [C:\src\DotNet\roslyn\src\EditorFeatures\CSharpTest\Microsoft.CodeAnalysis.CSharp.EditorFeatures.UnitTests.csproj]
C:\src\DotNet\roslyn\src\EditorFeatures\CSharpTest\Classification\SemanticClassifierTests.cs(1121,37): error IDE0055: Fix formatting [C:\src\DotNet\roslyn\src\EditorFeatures\CSharpTest\Microsoft.CodeAnalysis.CSharp.EditorFeatures.UnitTests.csproj]
C:\src\DotNet\roslyn\src\EditorFeatures\CSharpTest\Classification\SemanticClassifierTests.cs(1145,37): error IDE0055: Fix formatting [C:\src\DotNet\roslyn\src\EditorFeatures\CSharpTest\Microsoft.CodeAnalysis.CSharp.EditorFeatures.UnitTests.csproj]
C:\src\DotNet\roslyn\src\EditorFeatures\CSharpTest\Classification\SemanticClassifierTests.cs(1147,37): error IDE0055: Fix formatting [C:\src\DotNet\roslyn\src\EditorFeatures\CSharpTest\Microsoft.CodeAnalysis.CSharp.EditorFeatures.UnitTests.csproj]
C:\src\DotNet\roslyn\src\EditorFeatures\CSharpTest\Classification\SemanticClassifierTests.cs(1768,37): error IDE0055: Fix formatting [C:\src\DotNet\roslyn\src\EditorFeatures\CSharpTest\Microsoft.CodeAnalysis.CSharp.EditorFeatures.UnitTests.csproj]
    0 Warning(s)
    14 Error(s)

This PR correct all the errors.

@mikkelbu mikkelbu requested review from a team as code owners December 21, 2018 21:46
@jcouv
Copy link
Member

jcouv commented Dec 21, 2018

Tagging @sharwell
The intention at this point is that these should be warnings, not errors. I think the "treat warnings as errors" flags was already reverted.

jcouv
jcouv approved these changes Dec 21, 2018
View reviewed changes
Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM Thanks (iteration 1)

@jcouv jcouv added this to the 16.0.P2 milestone Dec 21, 2018
@jcouv jcouv self-assigned this Dec 21, 2018
@sharwell
Copy link
Member

@jcouv IDE0055 is treated as a warning in CI. It's still treated as an error in Build.cmd in local development scenarios.

/cc @tmat @jaredpar

@AlekseyTs
Copy link
Contributor

IDE0055 is treated as a warning in CI. It's still treated as an error in Build.cmd in local development scenarios.

I think that the real fix should be to make sure the formatting warnings are not treated as errors in Roslyn regardless of build environment.

@jaredpar
Copy link
Member

It's still treated as an error in Build.cmd in local development scenarios.

That was not the intent. Having it be an error in VS means that spaces, and other formatting errors, prevent F5. That was a scenario we specifically wanted to keep working. On my box this is how it functions (it is a warning for F5).

@sharwell
Copy link
Member

sharwell commented Dec 26, 2018 •
edited
Loading

@jaredpar it works correctly in the IDE, but Build.cmd reports the error. You can see it like this:

.\Restore.cmd
.\Build.cmd -rebuild

@AlekseyTs
Copy link
Contributor

it works correctly in the IDE, but Build.cmd reports the error.

This effectively prevents developers from running tests locally through the Build.cmd. For example, I see the build broken due to the warnings in some generated VB files:

F:\GitHub\roslyn2\roslyn\artifacts\obj\Microsoft.CodeAnalysis.VisualBasic.Scripting.UnitTests\Debug\net472\.NETFramework,Version=v4.7.2.AssemblyAttributes.vb(2,1): error IDE0055:
Fix formatting [F:\GitHub\roslyn2\roslyn\src\Scripting\VisualBasicTest\Microsoft.CodeAnalysis.VisualBasic.Scripting.UnitTests.vbproj]
F:\GitHub\roslyn2\roslyn\artifacts\obj\Microsoft.CodeAnalysis.VisualBasic.Scripting.UnitTests\Debug\net472\.NETFramework,Version=v4.7.2.AssemblyAttributes.vb(4,1): error IDE0055:
Fix formatting [F:\GitHub\roslyn2\roslyn\src\Scripting\VisualBasicTest\Microsoft.CodeAnalysis.VisualBasic.Scripting.UnitTests.vbproj]
F:\GitHub\roslyn2\roslyn\artifacts\obj\Microsoft.CodeAnalysis.VisualBasic.Scripting.UnitTests\Debug\net472\.NETFramework,Version=v4.7.2.AssemblyAttributes.vb(5,1): error IDE0055:
Fix formatting [F:\GitHub\roslyn2\roslyn\src\Scripting\VisualBasicTest\Microsoft.CodeAnalysis.VisualBasic.Scripting.UnitTests.vbproj]
F:\GitHub\roslyn2\roslyn\artifacts\obj\Microsoft.CodeAnalysis.VisualBasic.Scripting.UnitTests\Debug\net472\.NETFramework,Version=v4.7.2.AssemblyAttributes.vb(6,1): error IDE0055:
Fix formatting [F:\GitHub\roslyn2\roslyn\src\Scripting\VisualBasicTest\Microsoft.CodeAnalysis.VisualBasic.Scripting.UnitTests.vbproj]

There are many errors like this.

@sharwell
Copy link
Member

@AlekseyTs that's a different issue: #31799

@AlekseyTs
Copy link
Contributor

@sharwell I think the point that I am trying to make is that IDE0055 should never break Roslyn build, even when they are reported. Changing the code to make sure those warning are fixed is orthogonal. Developer should not be forced to fix them in order to make progress or be able to use Build.cmd.

@AlekseyTs
Copy link
Contributor

Therefore, I think this PR doesn't fix the problem with Build.cmd.

@AArnott
Copy link
Contributor

AArnott commented Dec 26, 2018

IMO this PR should be completed, and build.cmd should be fixed to allow these to be warnings rather than errors, since it's the only build environment in which they are errors.
I may produce a PR to fix build.cmd (since I'm hitting this myself). In the meantime, can someone complete this PR?

@AArnott AArnott mentioned this pull request Dec 26, 2018
Closed
AlekseyTs
AlekseyTs approved these changes Dec 27, 2018
View reviewed changes
Copy link
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (iteration 1)

@AlekseyTs
Copy link
Contributor

CC @jaredpar For approval

@sharwell sharwell merged commit bff68d6 into dotnet:master Dec 28, 2018
@mikkelbu mikkelbu deleted the clean-compile-without-IDE0055 branch December 28, 2018 06:45
@sandyarmstrong sandyarmstrong mentioned this pull request Dec 28, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jcouv jcouv jcouv approved these changes

@AlekseyTs AlekseyTs AlekseyTs approved these changes

@sharwell sharwell sharwell approved these changes

Assignees

@jcouv jcouv

Labels
Area-Compilers
Projects
None yet
Milestone
16.0.P2
Development

Successfully merging this pull request may close these issues.
6 participants
@mikkelbu @jcouv @sharwell @AlekseyTs @jaredpar @AArnott