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

[BUG] nx lint fails on dotnet project #179

Closed
dasco144 opened this issue Oct 5, 2021 · 9 comments · Fixed by #204
Closed

[BUG] nx lint fails on dotnet project #179

dasco144 opened this issue Oct 5, 2021 · 9 comments · Fixed by #204
Labels
bug Something isn't working outdated scope: core

Comments

@dasco144
Copy link
Contributor

dasco144 commented Oct 5, 2021

Describe the bug
The nx lint command fails on the dotnet project due to the check argument being removed in favour of verify-no-changes here

To Reproduce

  1. run nx lint on your dotnet project
  2. the command runs and installs dotnet-format (latest version at time of writing 5.1.225507)
  3. the command fails
  4. See error

Expected behavior
The command should run and not fail

Screenshots
image

Environment:

  • OS: Windows 10
  • Browser: N/A
  • Version: 1.4.2
  • Affected Packages: nx-dotnet

Additional context
When manually running the same command with check replaced with verify-no-changes, it works as expected

@dasco144 dasco144 added bug Something isn't working needs-triage This issue has yet to be looked over by a core team member labels Oct 5, 2021
@AgentEnder
Copy link
Member

Yikes, I didn't expect that option to change. That seems like a breaking change for no good reason on the dotnet side of things. I'll work on this at some point, need to make sure it works with the older versions too.

Thanks for the report.

@AgentEnder AgentEnder added scope: core and removed needs-triage This issue has yet to be looked over by a core team member labels Oct 5, 2021
@bcallaghan-et
Copy link
Collaborator

@dasco144 What version of the .NET SDK are you using? I'm using SDK 5.0.402. Using both dotnet-format 5.1.225507 and 5.1.250801, I see the --check flag rather than the --verify-no-changes flag.

$ dotnet format --version
5.1.250801+4a851ea9707f87d381166c2fc2b2d4b58a10a222

$ dotnet format --help
dotnet-format
  dotnet-format

Usage:
  dotnet-format [options] [<workspace>]

Arguments:
  <workspace>  A path to a solution file, a project file, or a folder containing a solution or project file. If a path is not specified then the current directory is used. [default: ]

Options:
  --no-restore                                                             Doesn't execute an implicit restore before formatting.
  -f, --folder                                                             Whether to treat the `<workspace>` argument as a simple folder of files.
  -w, --fix-whitespace                                                     Run whitespace formatting. Run by default when not applying fixes.
  -s, --fix-style <error|info|warn>                                        Run code style analyzers and apply fixes.
  -a, --fix-analyzers <error|info|warn>                                    Run 3rd party analyzers and apply fixes.
  --diagnostics <diagnostics>                                              A space separated list of diagnostic ids to use as a filter when fixing code style or 3rd party issues. [default: ]
  --include <include>                                                      A list of relative file or folder paths to include in formatting. All files are formatted if empty. [default: ]
  --exclude <exclude>                                                      A list of relative file or folder paths to exclude from formatting. [default: ]
  --check                                                                  Formats files without saving changes to disk. Terminates with a non-zero exit code if any files were formatted.
  --report <report-path>                                                   Accepts a file path, which if provided, will produce a json report in the given directory.
  -v, --verbosity <d|detailed|diag|diagnostic|m|minimal|n|normal|q|quiet>  Set the verbosity level. Allowed values are q[uiet], m[inimal], n[ormal], d[etailed], and diag[nostic]
  --binarylog <binary-log-path>                                            Log all project or solution load information to a binary log file.
  --version                                                                Show version information
  -?, -h, --help                                                           Show help and usage information

Similarly, using the --check flag worked with both versions, while using --verify-no-changes resulted in an error.

@dasco144
Copy link
Contributor Author

dasco144 commented Oct 14, 2021

Yes I think it is related to the dotnet 6 preview. I installed the Visual Studio 2022 preview and it installed the dotnet 6 preview with it, and the cli seems to use the latest installed version.

Dotnet format is being added to the cli with dotnet 6 so that seems to be the reason for the change (as per the note here)

@bcallaghan-et
Copy link
Collaborator

Thanks for the link to that thread. Having read it, it appears that v5 uses --check while v6 (bundled with the SDK) uses --verify-no-changes.

@AgentEnder The bundled version has several other breaking changes besides the one flag. How do you want to handle the migration from v5 to v6?

@AgentEnder
Copy link
Member

Hey @bcallaghan-et, yeah there are a few other breaking changes but I think thats the only one that directly impacts our defaults. For your PR I'd recommend adding some logic that detects if .NET 6 is the installed SDK, and if it is skip the tool install. If it is than also just swap the arg when being passed.

I'll take a closer look at all of the breaking changes in format and see if there is something we can do to help with a more full migration. With .NET 6 being a preview this isn't a huge deal yet, as long as the executors can still run.

Honestly, for now, I could see us detecting the .NET 6 SDK and warning and skipping the actual format, while we get this behavior worked out.

bcallaghan-et referenced this issue in Etogy/nx-dotnet Oct 14, 2021
Pass the correct "check" option depending on the SDK version

Fixes #179
AgentEnder added a commit that referenced this issue Oct 14, 2021
…and (#204)

Closes #179
Closes #202

Co-authored-by: Craigory Coppola <craigorycoppola@gmail.com>
Co-authored-by: Ben Callaghan <bcallaghan@selectbankcard.com>
github-actions bot pushed a commit that referenced this issue Oct 14, 2021
## [1.4.3](v1.4.2...v1.4.3) (2021-10-14)

### Bug Fixes

* **core:** Check SDK and tool installation before running format command ([#204](#204)) ([3ad6291](3ad6291)), closes [#179](#179) [#202](#202)

Oct 14, 2021, 11:24 PM
@github-actions
Copy link
Contributor

🎉 This issue has been resolved in version 1.4.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

@pemsbr
Copy link
Contributor

pemsbr commented Oct 15, 2021

@AgentEnder @bcallaghan-et I'm not able to lint my projects after this update, getting the following error:

> nx run project-a:lint
Executing Command: dotnet --version
5.0.400
Cannot read property 'toString' of null

I'm on version 5.0.400 and dotnet-format 5.1.225507

@AgentEnder
Copy link
Member

@pemsbr I'm going to break this out into a separate issue, I've identified problem and will get it resolved.

@github-actions
Copy link
Contributor

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working outdated scope: core
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants