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

C# analyzer and formatting #215

Merged
merged 10 commits into from
Jun 3, 2024
Merged

C# analyzer and formatting #215

merged 10 commits into from
Jun 3, 2024

Conversation

externl
Copy link
Member

@externl externl commented Jun 3, 2024

  • Add StyleCop
  • Format C# code
  • Fix many lints and recommendations

Still various things to fix:

  • Setting <AnalysisMode>All</AnalysisMode> causes many warnings
  • various dotnet format warnings:
Msbuild failed when processing the file '/Users/joe/Developer/zeroc-ice/ice-demos/csharp/Ice/callback/msbuild/client/client.csproj' with message: /Users/joe/.nuget/packages/zeroc.icebuilder.msbuild/5.0.9/build/zeroc.icebuilder.msbuild.csharp.targets: (53, 9): Ice Installation invalid or not detected. Invalid IceToolsPath setting `'
...
Unable to fix SA1600. Code fix SettingsFileCodeFixProvider doesn't support Fix All in Solution.
Unable to fix SA1600. Code fix InheritdocCodeFixProvider didn't return a Fix All action.

I tried to add dotnet format as a separate workflow but it failed to restore packages.

Run dotnet format --verify-no-changes "csharp/C# NET demos.sln"
Unhandled exception: System.Exception: Restore operation failed.
   at Microsoft.CodeAnalysis.Tools.CodeFormatter.OpenMSBuildWorkspaceAsync(String solutionOrProjectPath, WorkspaceType workspaceType, Boolean noRestore, Boolean requiresSemantics, String binaryLogPath, Boolean logWorkspaceWarnings, ILogger logger, CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.Tools.CodeFormatter.FormatWorkspaceAsync(FormatOptions formatOptions, ILogger logger, CancellationToken cancellationToken, String binaryLogPath)
   at Microsoft.CodeAnalysis.Tools.FormatCommandCommon.FormatAsync(FormatOptions formatOptions, ILogger`1 logger, CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.Tools.Commands.RootFormatCommand.FormatCommandDefaultHandler.InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken)
   at System.CommandLine.Invocation.InvocationPipeline.InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken)
Error: Process completed with exit code 1.

@externl externl requested a review from pepone June 3, 2024 18:02
Copy link
Member

@pepone pepone left a comment

Choose a reason for hiding this comment

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

Looks good, but seems there is still a few warnings to fix, making CI fail

// cts is canceled by Ctrl+C or a shutdown request.
// With C# 7.1 and up, you should make Main async and call: await Task.Delay(-1, cts.Token)
cts.Token.WaitHandle.WaitOne();
// cts is canceled by Ctrl+C or a shutdown request.
Copy link
Member

Choose a reason for hiding this comment

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

We should add a TODO or issue to fix this C# 7.1 is quite old.

Copy link
Member Author

Choose a reason for hiding this comment

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

I opened an issue

@pepone
Copy link
Member

pepone commented Jun 3, 2024

Run dotnet format --verify-no-changes "csharp/C# NET demos.sln"
Unhandled exception: System.Exception: Restore operation failed.

Probably because we didn't install the 3.8.0-alpha package, and it cannot be found in any of the default repositories.

@externl externl requested a review from pepone June 3, 2024 19:39
@externl externl merged commit 61eacaf into zeroc-ice:main Jun 3, 2024
5 checks passed
@externl externl deleted the csharp-analyzer branch June 3, 2024 20:02
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