Validation mode

[jaredpar]

The code formatter is great tool for getting code into the particular format desired by our guidelines. That is great for helping developers meet our style guidelines but it does nothing for telling developers that they are failing to meet them.

We should add a validation mode to this tool such that it can become a part of a CI system. A way to validate that new code doesn't violate the coding guidelines.

To be clear: I'm not proposing this feature be immediately integrated into any repo. I'm proposing we add the feature so that teams can decide whether or not they want strict enforcement on checkins.

[ellismg]

What are you envisioning for reporting here? Just an exit code that signals differences? Some sort of diff written to standard out?

[jaredpar]

@ellismg minimal implementation would just be a message + an exit code.

If we wanted to extend this mode later perhaps display the diffs of where the code needed to be changed.

[dannyvv]

@jaredpar;@ellismg I agree and would also like to see if there could be a validation mode. One that doesn't modify the sources and we can run as part of the regular build. I would love to see two modes for this:

1. Warn-mode: * Each violation would be reported as a warning. I.e. MyFolder\MyFile.cs(10,20): warning: Description of issue for use education. * The exit code would be 0. * This would be useful in the inner-dev loop: Provide early user education to dev, power squiggles in VS.
2. Error-mode: * Each violation would be reported as an error. I.e. MyFolder\MyFile.cs(10,20): error: Description of issue for use education. * The exit code would be 1. * This would be useful as part of the pull-request validation.

I don't have too much time. But I might be able to free some stuff up and pick up the work in the coming weeks (No guarantee though)

[jaredpar]

@dannyvv

Implementing validation mode is actually pretty straight forward functionally and could be done in a short amount of time. Essentially at the very last step instead of calling TryApplyChanges we enter into a diffing mode where we simply compare the textual content of every Document object in the set of DocumentId being considered for changes.

Implementing validation mode with good error messages though is trickier. There are certain rules for which generating a good error message is easy: private field rename, explicit this, etc ... Rules where we both identify and correct the problem.

But rules like just formatting the code aren't even really doable. We don't control the code in question there and simply rely on the operation being idempotent. It's not really possible to detect the error up front.

In some ways though maybe this is okay. The tool is designed to make formatting easy, not bug developers about individual rule. Perhaps a message that Document.cs isn't formatted correctly is enough. Giving detail about the error would imply that we wanted them to fix it vs. say "just run the formatter".

For context though we haven't really found a need for validation mode yet. The way I've been handing Roslyn is I just run the formatter once a week, validate the tests are passing and push the change. The only reason I even send the change out for review is because the IDE team is interested in inspecting the output to see if they've inadvertently introduced any errors into the code.

[dannyvv]

My idea was to create a ValidationLogger. Rules can report issues with a corresponding location info. Those reported will be printed to the user immediately Before saving We'll do a diff between old and new. For each line that doesn't have a rule report associated with it, report a generic issue with the old + new line. We can see how this works in practice and if we need to make the last step smarter and support diff regions.

I can't seem to self-assign this issue, but feel free to assign to me.

[jaredpar]

@dannyvv I'm curious what the goal of validation mode would be though? At the point the tool knows what the violation is, it can fix it immediately. What would be the value in giving the user an error when we could just fix it?

[jaredpar]

I do agree though this would be the most logical way to implement it. Specific where possible, diff where necessary.

JaredPar from a phone http://blog.paranoidcoding.com/

---

From: dannyvv [email protected] Sent: Monday, March 9, 2015 6:31:07 PM To: dotnet/codeformatter Cc: Jared Parsons Subject: Re: [codeformatter] Validation mode (#12)

My idea was to create a ValidationLogger. Rules can report issues with a corresponding location info. Those reported will be printed to the user immediately Before saving We'll do a diff between old and new. For each line that doesn't have a rule report associated with it, report a generic issue with the old + new line. We can see how this works in practice and if we need to make the last step smarter and support diff regions.

I can't seem to self-assign this issue, but feel free to assign to me.

Reply to this email directly or view it on GitHubhttps://github.com/dotnet/codeformatter/issues/12#issuecomment-77979803.

[dannyvv]

Short:

· I Want to be educated early on about what I am doing wrong

· I Don’t want to be the one to always be cleaning up others stuff.

I have certain coding guidelines hard-baked into my fingers. I work on various projects with various guidelines and it is impossible to remember all the details of each codebase. I have always managed to configure resharper for those projects in such a way that auto-cleanup from VS would get the code to the correct guidelines of the team.

It is always a pain when the guideline are not enforced somewhere because with this way of working I always end up having too many changes not related to the core fix and carry massive merge conflict penalties because of them. I therefore would like the tool to run as part of the checkin validation (either a unittest if we have to be on github, or a proper gate on the internal systems) Just to ensure nobody checks in bad formatting to protect me and my team members.

I want this validation to run as part of the build, so we get early feedback on what we are doing wrong with proper squiggles in VS. So over time we can start typing new code that follows the guideline.

I don’t want to auto-fixup in the regular build either because a clean build system only writes to the obj and src tree. And never ever touches the source tree. So the fixup should be done by running external command.

Thanks, Danny

From: Jared Parsons [mailto:[email protected]] Sent: Monday, March 9, 2015 6:33 PM To: dotnet/codeformatter Cc: Danny van Velzen Subject: Re: [codeformatter] Validation mode (#12)

@dannyvvhttps://github.com/dannyvv I'm curious what the goal of validation mode would be though? At the point the tool knows what the violation is, it can fix it immediately. What would be the value in giving the user an error when we could just fix it?

— Reply to this email directly or view it on GitHubhttps://github.com/dotnet/codeformatter/issues/12#issuecomment-77979938.