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

--check along with --dry-run not working as expected #113

Closed
francisminu opened this issue Apr 22, 2019 · 10 comments
Closed

--check along with --dry-run not working as expected #113

francisminu opened this issue Apr 22, 2019 · 10 comments

Comments

@francisminu
Copy link

francisminu commented Apr 22, 2019

Hi,

My intention was to get a set of files and the errors associated with them with the --check command. But seems like --check returns just a non-zero response if there are any files that do not conform to the rules.

I tried the same, but the command 'dotnet format --check' doesn't seem to be different from running dotnet format without any options. They both format the file and save changes to disk.

Now, I tried to run the option --dry-run --check (as mentioned in another issue), so that it shows just the files that have error and returns a non-zero exit code. But, the command executed successfully.

Please find below the code against which the command was run:

       class Program
       {
          static void Main(string[] args)
         {
            test();
            Console.WriteLine("Inside Main");
         }
         public static void test()
         {
            var i = 0;
            if (i == 0){
                Console.WriteLine("i is 0");
            }
            else
            {
                Console.WriteLine("i is not 0");
            }
            Console.WriteLine("Inside Test method");
         }
       }

Rule:
csharp_new_line_before_else = true
csharp_new_line_before_open_brace = all

When I ran > dotnet format --check
The braces were formatted in the file Program.cs and the changes were saved.

Now, I tried running > dotnet format --dry-run --check
With the hope that it will exit with non-zero exit code. But it showed the below message:

Formatting code files in workspace 'C:\Project\MyHobbyProjects\EditoConfigCLISample\ConsoleApp1\ConsoleApp1.sln'.
Loading workspace.
Workspace loaded in 1785ms.
Formatting code files in project 'ConsoleApp1'.
Formatting code file 'Program.cs'.
Formatted code file 'Program.cs'.
Formatted 1 of 3 files in 1275ms.
Format complete.

Is it possible to display the files that doesn't abide by EditorConfig and the rules that it does not conform to?

Thanks!

@jmarolf
Copy link
Contributor

jmarolf commented Apr 22, 2019

@francisminu can you post what the formatted file looks like? what does the formatter do with these settings?

@francisminu
Copy link
Author

francisminu commented Apr 22, 2019

When I run just --check, the files are formatted (just like running dotnet format without any options) and the response is as below:

    class Program
    {
        static void Main(string[] args)
        {
            test();
            Console.WriteLine("Inside Main");
        }

        public static void test()
        {
            var i = 0;
            if (i == 0)
            {
                Console.WriteLine("i is 0");
            }
            else
            {
                Console.WriteLine("i is not 0");
            }
            Console.WriteLine("Inside Test method");
        }
    }

When I run --dry-run --check, the files are not formatted, but the message in CLI is as below and the code is given below it:

Formatting code files in workspace 'C:\Project\MyHobbyProjects\EditoConfigCLISample\ConsoleApp1\ConsoleApp1.sln'.
Loading workspace.
Workspace loaded in 1497ms.
Formatting code files in project 'ConsoleApp1'.
Formatting code file 'Program.cs'.
Formatted code file 'Program.cs'.
Formatted 1 of 3 files in 1061ms.
Format complete.

Code:

    class Program
    {
        static void Main(string[] args)
        {
            test();
            Console.WriteLine("Inside Main");
        }

        public static void test()
        {
            var i = 0;
            if (i == 0){ 
                Console.WriteLine("i is 0");
            }
            else
            {
                Console.WriteLine("i is not 0");
            }
            Console.WriteLine("Inside Test method");
        }
    }

The console shows the same message in either case. But while running --dry-run --check together, the file is not modified.

Thanks!

@francisminu
Copy link
Author

@jmarolf Hi Jonathan,
Any update on this one?

Thanks!

@jmarolf
Copy link
Contributor

jmarolf commented Apr 23, 2019

Haven't had time to look at this yet @JoeRobich whitespace rules are always applied correct?

@ghost
Copy link

ghost commented Apr 24, 2019

If you don't mind, I think I can provide some input on this since I wrote some of the logging code. The --dry-run option specifies that changes are not saved to disk. The log messages that say files were formatted, I believe, are output after changes are discovered but before the program decides if it needs to actually save those changes to disk. The log messages may simply need to be refactored to happen at a different time.

As for some of the other points:

  1. if --dry-run --check is not returning a non-zero exit code, that sounds like a bug to me.
  2. I don't believe there's a way to output the specific rules that fail currently, but one of the maintainers can provide a more definitive answer on that.

@AnthonyMastrean
Copy link

AnthonyMastrean commented May 9, 2019

I installed this version

PS> dotnet format --version
3.0.4+211cab024c37fdffa5955c5855f9d62a14703452

And ran the same command with some extra status reporting at the end

PS> dotnet format --check --dry-run; $?; $LASTEXITCODE
  Formatting code files in workspace 'C:\Users\anthony\example\Example.sln'.
  Formatting code files in project 'Example'.
  Formatted code file 'Foo.cs'.
  Formatted code file 'Bar.cs'.
  Formatted code file 'Baz.cs'.
  ...
  Format complete.
False
1

And I double-checked the git status just to be sure that check didn't change any files (adhering to the dry-run flag).

PS> git status
On branch master
Your branch is up to date with 'origin/master'.

nothing to commit, working tree clean

@arnavb
Copy link

arnavb commented May 18, 2019

IMO, this behavior doesn't really make sense. Other autoformatters, like prettier or black (for JS and Python respectively), print out an error message instead of actually changing files. I feel like this tool should follow the same convention.

@JoeRobich
Copy link
Member

@AnthonyMastrean Thanks for giving it a run. That is the expected behavior. The formatter is run against each file and the files with formatting changes are printed to the console as Formatted code file 'Foo.cs'. Because files were formatted a non-zero exit code was returned as expected.

@arnavb I think the story around logging formatting issues is one that we could improve on. Perhaps an easy change to give more meaningful feedback would be to Log the Formatted code file 'Foo.cs'. message as Error instead of Info when the check flag is passed.

@JoeRobich
Copy link
Member

JoeRobich commented Jul 13, 2019

@arnavb With #249, we now log individual messages for each formatting issue. These are printed during --dry-runs and when you also include the --check flag it will log as an Error instead of a Warning.

You can try this change by installing from myget

dotnet tool install -g dotnet-format --version 3.1.36303 --add-source https://dotnet.myget.org/F/format/api/v3/index.json

@JoeRobich
Copy link
Member

Closing issue due to inactivity.

vdurante pushed a commit to vdurante/format that referenced this issue Feb 29, 2024
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

No branches or pull requests

5 participants