Editorconfig : Can't exclude const fields

[vsfeedback]

If a editorconfig with naming rules which defines that all privat fields should begin with an underscore and casing should be camel case.

I also define constants have to be uppercase.

The problem is, that VS shows me a warning because my private const does not begin with an underscore.

EditorConfig:

# General naming convention
[*.{cs,vb}] 

# internal and private fields should be _camelCase
dotnet_naming_rule.camel_case_for_private_internal_fields.severity = warning
dotnet_naming_rule.camel_case_for_private_internal_fields.symbols  = private_internal_fields
dotnet_naming_rule.camel_case_for_private_internal_fields.style    = camel_case_underscore_style

dotnet_naming_symbols.private_internal_fields.applicable_kinds = field
dotnet_naming_symbols.private_internal_fields.applicable_accessibilities = private, internal, friend
dotnet_naming_style.camel_case_underscore_style.required_prefix = _
dotnet_naming_style.camel_case_underscore_style.capitalization = camel_case 

# name all constant fields using UpperCase
dotnet_naming_rule.constant_fields_should_be_upper_case.severity = warning
dotnet_naming_rule.constant_fields_should_be_upper_case.symbols  = constant_fields
dotnet_naming_rule.constant_fields_should_be_upper_case.style    = upper_case_style

dotnet_naming_symbols.constant_fields.applicable_kinds   = field
dotnet_naming_symbols.constant_fields.required_modifiers = const
dotnet_naming_style.upper_case_style.capitalization = all_upper

const to test:

private const string CLIENT_PROPERTYKEY_REQUIRED_AD_GROUPS = "RequiredADGroups";

This issue has been moved from https://developercommunity.visualstudio.com/content/problem/152288/editorconfig-cant-exclude-const-fields.html
VSTS ticketId: 526669
These are the original issue comments:

Jinu Joseph [MSFT] on ‎11‎/‎27‎/‎2017, 03:55 AM (78 min ago):

We appreciate you taking the time to report this problem. We are currently prioritizing problems that are impacting a broad set of our customers, so we may not be able to investigate this one immediately. We know this problem is important to you, so we will continue to monitor it.

These are the original issue solutions:
(no solutions)

[jmarolf]

@dpoeschl for fact checking...

I do not believe we have the concept of rule hierarchy anymore in the naming style engine even if there was a way to express it in the parser.

In the snippet below camel_case_for_private_internal_fields applies and cannot be overridden by any other nameing style rule.

private const string CLIENT_PROPERTYKEY_REQUIRED_AD_GROUPS = "RequiredADGroups";

To fix this we will first need to implement the concept of rule hierarchies and then pass them through to the editorconfig format

[jmarolf]

There are two rules: one that applies to private fields and one that applies to const fields. The rule engine does not have a way for one rule to completely override another. Instead it applies them in order. The behavior is by-design right @dpoeschl? or is there something I'm missing?

[jmarolf]

The user is asking for constant_fields_should_be_upper_case to override camel_case_for_private_internal_fields in the case where we have a private const field. Both constant_fields_should_be_upper_case and camel_case_for_private_internal_fields apply here so there would first nned to be logic that naming styles can over-rule one another according to some spec, then we can add the APIs necessary to meet that spec.

The underlying API does not expose the property order, so there is no way to apply them "in order".

No, but the underlying implementation adds items to the AllRawConventions in a deterministic order that ends up being the order in which they are encountered. This is an implementation detail and not an API contract so we'd want to improve here as we come up with a spec for rule overriding (if that is a feature we want to do).

[dpoeschl]

The idea was that they would be applied in order, similar to how they are when configured from the UI. If that were working (or if it's working by coincidence somehow), then the original post should place the rule for constant fields above the rule for other fields (most specific first).

[dpoeschl]

Ignoring .editorconfig for a second, the super original implementation of this had a full tree of rules, with sub-rules and overrides and etc. We decided it was an easier mental model to just have a flat list that applies in order. You can put your more specific rules first and "mimic" overriding that way. We couldn't think of any scenarios that wouldn't work in this new, flat, paradigm.

Now, @sharwell does point out that the dictionary used in the .editorconfig parsing doesn't guarantee order, I also think @jmarolf might be right that the order is preserved by implementation. If this is all true, then reversing the order of the rules in the original post (more specific first) should fix the problem.

[jmarolf]

Alright, I discussed this scenario with @dpoeschl and I was wrong. This scenario will work today if you reverse the ordering of naming style declarations such that constant_fields_should_be_upper_case comes before camel_case_for_private_internal_fields. The question is: how is the user supposed to know that?

[chm-tm]

@jmarolf Just read the documentation, second paragraph. It's there since 2017-08-04.

[sharwell]

@jmarolf @dpoeschl It looks like the implementation of Dictionary<TKey, TValue> will enumerate items in insertion order as long as no item is ever removed. However, this behavior doesn't appear to be documented anywhere and isn't a requirement of the dictionary interfaces.

As for applying items in a top-down manner (first match wins), this appears to be the case users would expect most of the time. However, there are two areas where this applies:

1. When two naming rules appear in the same file (both match the code, and they have different names), it makes sense for the first one in the file to apply to the code.
2. When two naming rules appear in different files (still both match and have different names), it makes sense for the one in the file closest to the code to apply.

[jmarolf]

However, this behavior doesn't appear to be documented anywhere and isn't a requirement of the dictionary interfaces.

I agree this is depending on implementation specific behavior and therefore is bad. It is, however, extremely unlikely that corefx will ever make a change in behavior here.

@sharwell 1 and 2 describe the behavior of the feature today.

Since (as @chm-tm points out) this behavior is already documented behavior I think this is by design.

[Burtch]

I'm running into a similar issue. Perhaps you could add in the ability to say "dotnet_naming_symbols.constant_fields.required_modifiers = !const" or !readonly in my case. I want a rule to apply to any private member that ISN'T a readonly, (which would be _camelCase) and then for public or private readonly just apply a different rule, which would be (PascalCase)

I have the rules in order of most specific to least specific as suggested. However if I start by saying private int _myInt = 2, and then immediately edit that to include private readonly int _myInt = 2, the readonly inclusion won't be picked up because the first rule already sufficed.

If I were to just start off typing private readonly int _myInt = 2, my readonly rule would correctly be applied and appropriately give a warning. So the rules work, unless you start with just int and add in readonly after the rule was picked up....then it stops caring.

[Tadimsky]

I see the PR has now been merged - great!
Does this mean we are able to support this in editorconfig? When will that happen?

[CyrusNajmabadi]

@Tadimsky the only merge was of #25169. And that's just a generic algorithm that has been merged into a feature branch. It hasn't made it into the master branch, nor has it been adopted into the the editor_config area code.

[Tadimsky]

Ah - I should have looked at the PR 😢
In that case, is there any way for me to do what this issue describes?

[CyrusNajmabadi]

You'd have to start with:

    public interface ICodingConventionsSnapshot
    {
        IUniversalCodingConventions UniversalConventions { get; }
        IReadOnlyDictionary<string, object> AllRawConventions { get; }

AllRawConventions is how the data flows into roslyn. You'd need to track down how this flows everywhere, specifically into the NamingPreference code. That code will have to enumerate this dictionary looking for the first item that matches (instead of whatever it is doing now). Note: this will be us taking a strong dependency on that Dictionary enumerating things in an order we want. that's not really safe, but the only alternative is to get VS (the owners of that interface above) to give us the data in ordered form. For example, an ImmutableArray<(string key, object value)> OrderedRawConventions.

[CyrusNajmabadi]

@olegtk Would it be hard to get ICodingConventionsSnapshot to expose the raw conventions in an ordered fashion? Right now we're in the unpleasant state where we're going to need to take a dependency on how standard .net Dictionary iterates values, and that's... not a happy place to be in.

Thanks!

[olegtk]

@CyrusNajmabadi hmm, so you need to preserve original order? In general it makes sense, but we need to decide how do we deal with merging, for example .editorconfig in a repo root declares
propA = foo
propB = bar
and then another .editorconfig in a project folder says
propC = baz
propA = bax

How show we order resulting properties for files in that project? .editorconfigs are merged top to bottom so I guess:
propB = bar
propC = baz
propA = bax

Or maybe we should provide an extensibility point for you to customize the merging strategy? Or plainly expose pre-merged hierarchy?

[CyrusNajmabadi]

Or plainly expose pre-merged hierarchy?

This might be the most ideal. I think the roslyn strategy would then be to just examine the pre-merged hierarchy, walking upwards trying to find the first match in each each set.

Note: i really haven't thought this through. I could see confusion with any sort of strategy taken. @sharwell , thoughts?

[CyrusNajmabadi]

tagging @agocke as well who is porting hte editor code down to the compiler layer. If that work gets done, roslyn probably won't need the editor to do anything. however, any decisions we make should at least be applied to the CodeAnalysis implementation.

[tannergooding]

Basically copying my post from #26253 (comment)

EditorConfig files are imported and parsed from root downward (to the current directory). The standard behavior according to the editorconfig "spec" is last definition wins. My opinion is we should ensure that we follow this ordering, both to remain inline with the spec, but also to match what users familiar with editorconfig will expect.

When opening a file, EditorConfig plugins look for a file named .editorconfig in the directory of the opened file and in every parent directory. A search for .editorconfig files will stop if the root filepath is reached or an EditorConfig file with root=true is found.

EditorConfig files are read top to bottom and the closest EditorConfig files are read last. Properties from matching EditorConfig sections are applied in the order they were read, so properties in closer files take precedence.

[CyrusNajmabadi]

And my followup:

Let me rephrase: the IDE wants to use order to define some behavioral contract. Whether that is "first wins" or "last wins" and how that interacts with rules that are specified from outside-in, still needs to be committed to. If you want, you can def provide feedback on the linked bug where this is being discussed. The critical bit for IDE is that the API for obtaining this information be rich enough to at least supply whatever contract is decided upon, without depending on undefined behavior.

[olegtk]

@aditya-kadam FYI - a feature request