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

fix: compiler-version (and other) warnings #69

Merged
merged 5 commits into from
Sep 4, 2024

Conversation

wlsnmrk
Copy link
Contributor

@wlsnmrk wlsnmrk commented Sep 3, 2024

Just as a heads-up, this change moves the code fix into a separate package (explanation below), which may warrant some additional scrutiny before merging.

Analyzers/generators in LogicBlocks depend on v4.12.0-final of Microsoft.CodeAnalysis packages. That causes the resulting DLLs to reference v4.12.0 of the compiler, which is newer than v4.11.0 currently shipped. That in turn causes a CS9057 warning both when compiling LogicBlocks projects that depend on the analyzers, and in user projects referencing LogicBlocks:

Screenshot 2024-09-03 155853

To fix this, I've rolled back those Microsoft.CodeAnalysis references to v4.11. This causes the compiler to warn that analyzers and generators should now have the project property EnforceExtendedAnalyzerRules turned on:

Screenshot 2024-09-03 160135

After enabling that property, other warnings are generated, notably RS1038 in LogicBlocks.Analyzers and RS1035 in LogicBlocks.DiagramGenerator:

RS1038

Screenshot 2024-09-03 163359

A (new?) rule provides that analyzers and code fixes shouldn't exist in the same assembly, since code fixes rely on the Workspaces package, which is not available for command-line builds. To rectify this, I've moved the code fix into a separate assembly, LogicBlocks.CodeFixes. Additionally, I've put a reference to the code-fix assembly in the Example project, and I've also included it into the main LogicBlocks package.

I've used ProjectReferences in one of my own local projects to verify that IntelliSense and the compiler still pick up the error about the missing [LogicBlock] attribute with the split-out code-fix package:

Screenshot 2024-09-03 165634

RS1035

This warns that analyzers should not do file I/O. Since that's the whole purpose of the DiagramGenerator project, I don't see a fix or a workaround for this. Therefore I've just suppressed it in the project settings. (There's also an instance of the same warning code enforcing that analyzers should not access Environment properties, referring to the use of Environment.NewLine. However, issue 6467 at the dotnet/roslyn-analyzers project contains an open discussion about whether NewLine should be an exception, so I haven't changed anything there.)

RS1039

This was warning that the Tester.GetSymbol method, in GeneratorTester, invoked SemanticModel.GetDeclaredSymbol on a SyntaxNode, which always returns null for certain subclasses of SyntaxNode. However, that code was unused, so I've removed it rather than address nullability or anything.

Note: this commit is a partial fix and still throws warnings at build

* Rolled back Microsoft.CodeAnalysis.* refs to 4.11 to fix warning
  in dependent projects CS9057, "analyzer assembly references version
  4.12.0.0 of the compiler, which is newer than the currently running
  version"
* Added property "EnforceExtendedAnalyzerRules" to analyzer projects
  to satisfy RS1036
Note: This commit still throws warnings during compilation.

* Resolve RS1038 (compiler extensions in an assembly referencing
  Microsoft.CodeAnalysis.Workspaces) by moving code fixes into a
  separate analyzer project and removing the reference to Workspaces
  from LogicBlocks.Analyzers.
* Reference the CodeFixes assembly from Example.
* Direct the LogicBlocks project to pack in the CodeFixes assembly
  during compilation.
Note: This commit still throws warnings on compilation.

Resolve RS1039 (SemanticModel.GetDeclaredSymbol will always return
null for certain subclasses of SyntaxNode) by removing unused
parsing code in GeneratorTester
The diagram generator is deliberately doing file i/o in an analyzer,
which causes the compiler to emit RS1035. Since the violation is
deliberate and doesn't admit a workaround, suppressed the warning at
the project level.
Copy link
Member

@jolexxa jolexxa left a comment

Choose a reason for hiding this comment

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

Thank you SO much for doing this 🥹

@jolexxa jolexxa changed the title Fix compiler-version (and other) warnings fix: compiler-version (and other) warnings Sep 4, 2024
@jolexxa jolexxa merged commit 73f43ab into chickensoft-games:main Sep 4, 2024
2 checks passed
@wlsnmrk wlsnmrk deleted the compiler-version-fix branch September 4, 2024 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants