Expose CSharpCodeStyleOption in API

[denis-prodan]

At this moment it seems to be impossible to get style options for C# in code fixer.
For example, I can get few generic CodeStyleOptions or CSharpFormattingOptions, but can't get CSharpCodeStyleOptions.
This behavior complicates the development of some extensions. For example:

if (param1 == null)
    throw new ArgumentNullException(nameof(param1));

or

if (param1 == null)
{
    throw new ArgumentNullException(nameof(param1));
}

It could be really useful if analyzer automatically detects PreferBraces option and decide how to build a statement. I see some usage examples inside of Roslyn, but it is not an option when trying to do that outside.
I understand that there could be implications to do that, or maybe even another way to achieve my goal. If yes, could you please elaborate it?

[CyrusNajmabadi]

The expected form here is to not try to do this manually, but to just put teh appropriate annotations on things and allow the formatter to format the document after you've made your changes. This happens automatically if you use the Analyzer/CodeAction infrastructure. Specifically, it happens here:

roslyn/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs

Lines 264 to 281 in e086a1d

	internal static async Task<Document> CleanupDocumentAsync(
	Document document, CancellationToken cancellationToken)
	{
	if (document.SupportsSyntaxTree)
	{
	document = await Simplifier.ReduceAsync(document, Simplifier.Annotation, cancellationToken: cancellationToken).ConfigureAwait(false);
	// format any node with explicit formatter annotation
	document = await Formatter.FormatAsync(document, Formatter.Annotation, cancellationToken: cancellationToken).ConfigureAwait(false);
	// format any elastic whitespace
	document = await Formatter.FormatAsync(document, SyntaxAnnotation.ElasticAnnotation, cancellationToken: cancellationToken).ConfigureAwait(false);
	document = await CaseCorrector.CaseCorrectAsync(document, CaseCorrector.Annotation, cancellationToken).ConfigureAwait(false);
	}
	return document;
	}

Trying to do this yourself is actually quite complex as there is a lot involved with any formatting rule. If you try to do it yourself you will effectively just be trying to replicate all the logic already baked into the formatting engine and the existing formatting rules.

[denis-prodan]

How I can achieve that?
Practically, I'm creating analyzer and fixer that literally doing same as in the example above (check that parameters in a constructor are not null and add these checks in fixer).
So, I'm creating a changed statement list in a constructor body and then replace body node with documentEditor.ReplaceNode();
Result formatted perfectly (in terms of line breaks and indentation), except braces - they're always unchanged regardless of that setting. I've tried Simplifier (both Reduce and Expand methods) without any success in that area as well.

[CyrusNajmabadi]

How I can achieve that?

By calling Formatter.FormatAsync(document, Formatter.Annotation, cancellationToken: cancellationToken) and Formatter.FormatAsync(document, SyntaxAnnotation.ElasticAnnotation, cancellationToken: cancellationToken) as per the example i showed above.

I've tried Simplifier (both Reduce and Expand methods) without any success in that area as well.

Simplifying is unnecessary here. As mentioned, you need to use the formatter to ensure proper formatting.

[denis-prodan]

Unfortunately, it doesn't work for me
Here is the method:

        private async Task<Solution> AddNullCheck(Document document, ConstructorDeclarationSyntax constructor, IList<string> paramNames,  FixType fixType, CancellationToken cancellationToken)
        {
            var newBodyStatements = GetNewBodyStatements(constructor, paramNames, fixType);

            if (!newBodyStatements.Any())
            {
                return document.Project.Solution;
            }

            var newBody = constructor.Body.WithStatements(newBodyStatements);

            var documentEditor = await DocumentEditor.CreateAsync(document, cancellationToken);
            documentEditor.ReplaceNode(constructor.Body, newBody);
            var newDocument = documentEditor.GetChangedDocument();

            newDocument = await Formatter.FormatAsync(newDocument, Formatter.Annotation, cancellationToken: cancellationToken);
            newDocument = await Formatter.FormatAsync(newDocument, SyntaxAnnotation.ElasticAnnotation, cancellationToken: cancellationToken);
            return newDocument.Project.Solution;
            //return await AddMissingUsingsIfNeeded(documentEditor, newDocument, GetRequiredUsingName(fixType), cancellationToken);
        }

Fixer preview:

Resulting code with warning and fixer to add braces to if (to show that my settings has "prefer braces = true"):

[CyrusNajmabadi]

Oh... i got confused about your title. You said "CSharpFormattingOptions". But 'PreferBraces' is a CSharpCodeStyleOption.

I agree we should have some mechanism for the right thing to be done here. Note that 'PreferBraces' is poorly named. It's really more about 'Require Braces'. So regardless of the setting, we always put in braces. It's just htat if the value is 'false' the user isn't told there's a problem if they're missing.

So, for your fix, just always add the braces.

[CyrusNajmabadi]

Have filed #29621 over hte confusion about the name 'PreferBraces'.

[denis-prodan]

Sorry, copy-pasted wrong class name into the title. I'll change it to make more descriptive.
Yeah, it is kind of workaround to always add braces, but I'm pretty sure that some people won't like that because their style guides really prefer to have one-liners without braces (and it is completely legit to want it here).
It would be hard to explain that it is wrongly named option in VS, not fixer issue :(

[CyrusNajmabadi]

It would be hard to explain that it is wrongly named option in VS, not fixer issue :(

Considering that nothing in VS supports that option or tries to enforce that... it would be hard to say it's a problem for you to behave entirely like the rest of VS :)

[denis-prodan]

So, just to clarify - there are no plans to expose CSharpCodeStyleOption right now, all settings should be applied automatically?

[CyrusNajmabadi]

Is there a reason you need CSharpCodeStyleOption exposed? Could you clarify what you would use it for?

[denis-prodan]

Practically, any fixers that touching something affected by code style settings.
In my example, I can just add a simple "if" statement without braces, until another preference is not specified. Just to add 2 lines of code instead of 4 (which could be useful).
Like ReSharper does:


It has support for EditorConfig settings.

Or something like "prefer expression body for methods/constructors/operators". Some fixers could have one-liners and it good to know if it is a preferred way to have expression body or fixer should generate more "old school" code.
Or add some local variables with "var" or type name (affected by UseImplicitTypeWherePossible). Of course, fixer can always use explicit types, but I have a feeling that it is not very convenient for users because they could have "var" as default convention (or at least assume that, unless they specify opposite).

So, practically, it is a question of being able to create more convenient fixers, that could do exactly what user wants to have, not only some default (or "safe") behavior.

[CyrusNajmabadi]

In my example, I can just add a simple "if" statement without braces

As mentioned, there is no IDE preference controlling that. Roslyn behavior is to always use braces. There is only an option stating how severe a problem it is if no braces are used. But there is no preference to say "do not use braces". The preference is always "use braces" :)

Some fixers could have one-liners and it good to know if it is a preferred way to have expression body or fixer should generate more "old school" code.

The expectation is that fixers should not have to be aware of this. They should just generate the new code and the Roslyn code fix infrastructure will fix things up to match user preference. Note that we do this now with the 'var vs explicit-type' preference. The plumbing has not gone through to support expression-bodies yet, but it's on list to do at some point.

The benefit here is that it's now no longer necessary for each feature to have to figure this out. They should just be able to do something like make a method, and if it's got the right annotation on it, the code-fix system will just convert it to match user style.

Or add some local variables with "var" or type name

As mentioned, if you create code with types in it now (like a local-declaration), the IDE will attempt to make it match the user's code-style preference. I believe this happens automatically if you use SyntaxGEnerator, and you can opt-into it by adding the Simplification annotation on a local-declaration if you create one manually.

[CyrusNajmabadi]

So, practically, it is a question of being able to create more convenient fixers, that could do exactly what user wants to have, not only some default (or "safe") behavior.

I strongly agree with that. However, i don't think the right approach here is to expose options. Options aren't that helpful here because now the fix author has to still do all the work necessary to figure out what to do with teh varying options. For example, say the "use var" option is on, how does a fix-author still find out if/when 'var' would be legal in tihs case?

Or say the option is set to "use var when type is apparent". How does a fix-author know if the type is apparent in this case? The right way IMO, is to have the fix author opt-in (or potentially opt-out) and have the Roslyn system handle this itself taking into account the options as well as all the accumulated smarts it has built up in these areas over its lifetime.

--

Note: say we did finally have an option to "do not have braces if not necessary". Then i would want Roslyn's fix infrastructure to apply that to any annotated node. That way people could just make fixers that created if-statements, and they would all come out according to the user's preferences, without every fix-author having to read and support this option.

[denis-prodan]

It would be great to have automatically applied style settings.
However, it is not 100% clear for me how it is supposed to work.
Let just assume that we have options same as right now ("Require braces").
In this case, it looks logical to create a statement without braces and let Roslyn add braces automatically in case of need. Otherwise, it is impossible (in terms of current settings) to determine if a version with braces made intentionally or like "safe" option (since this is "Prefer/Require braces", not "Prefer statements without braces").
Same for expression-bodied members - create them by default and let Roslyn (in case of need) convert to a full-bodied implementation?

Or, there are plans to changes settings in a way to allow Roslyn change any code to the desired style? I have feeling that this is not a simple task, and probably not possible to have backward compatibility with current settings set (like "Require braces" is not compatible with "prefer no braces for one-liners").

UPDATE
I just updated VS to the latest version and there is setting "add/remove braces for single-line control statement". When I press Ctrl + E, D (but not Ctrl E, F by some reason), it reformats code as I said - add braces if they're not added (but not removing them if they're not required, according to "Require braces" name). So, are you planning to apply this functionality to fixers result code? Or maybe there is some way to do that right now?

[CyrusNajmabadi]

It would be great to have automatically applied style settings.
However, it is not 100% clear for me how it is supposed to work.

The idea is this: as the code-fixer, you generate the code in either style (usually the more verbose style, since you can think about less stuff). i.e. if you working with ifs, always use braces. If you're working with methods, just always have an block body. Them put the Simplification annotation on the nodes you create. The Simplifier will then say: "teh code creating this want this updated to match the user's style, let me see what i can do".

Note: this is already what we do today. If you use SyntaxGenerator.TypeExpression(someTypeSymbol) we'll generate a name like global::System.Collections.Generic.List<global::System.Int32>. But it is annoted, and hte later passes will turn that into List<int> (or even var if appropriate).

[CyrusNajmabadi]

Note: It migt be good to consider a different annotation as well. i.e. CodeStyle.Annotation instead of Simplifer.Annotation. The reason for this (for me at least) is that the 'Simplifier' annotation has the semantics of applying to all nodes under an annotated node. It's unclear to me if we'd want that for CodeStyle. i.e. if you wrapped some existing code with an 'if', it's fine to say "the 'if' should match the user's code style", but it doesn't mean you want all nested code to be updated.

[CyrusNajmabadi]

UPDATE
I just updated VS to the latest version and there is setting "add/remove braces for single-line control statement". When I press Ctrl + E, D (but not Ctrl E, F by some reason), it reformats code as I said - add braces if they're not added (but not removing them if they're not required, according to "Require braces" name). So, are you planning to apply this functionality to fixers result code? Or maybe there is some way to do that right now?

Interesting! Sounds like something worth doing. I'll see if i have time over the next couple of weeks to do this. Prefer-Braces is small enough and nicely contained enough to likely make this a good test candidate for this design.

[denis-prodan]

Thank you! Please, update this thread so I'll be able to try it