Skip to content

Analyze naming rules#71

Merged
madskristensen merged 3 commits intomadskristensen:masterfrom
sharwell:analyze-naming-rules
Jun 14, 2019
Merged

Analyze naming rules#71
madskristensen merged 3 commits intomadskristensen:masterfrom
sharwell:analyze-naming-rules

Conversation

@sharwell
Copy link
Copy Markdown
Collaborator

@sharwell sharwell commented Jun 13, 2019

The compiler implementation of .editorconfig changes the way naming rules are ordered. This pull request implements an analyzer to report a warning when the .editorconfig file contains two rules that will be evaluated in reverse order from their location in the .editorconfig file.

The warning emitted for this situation reads as follows:

Naming rule '{0}' was reordered relative to overlapping rule '{1}'. Elements which match both will be validated against rule '{0}' starting with Visual Studio 2019 version 16.3.

sharwell added 2 commits June 13, 2019 15:18
* Modifiers match before accessibility
* 'const' always includes 'static' and 'readonly'
Comment thread src/Resources/Text.resx
<value>"{0}" is not a valid value for the "{1}" property</value>
</data>
<data name="NamingRuleReordered" xml:space="preserve">
<value>Naming rule '{0}' was reordered relative to overlapping rule '{1}'. Elements which match both will be validated against rule '{0}' starting with Visual Studio 2019 version 16.3.</value>
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 This might actually be 16.2... will know for sure soon

}
}

private bool Overlaps(in NamingRule x, in NamingRule y)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 This implementation doesn't currently handle cases where the ApplicableSymbolKindList and RequiredModifierList are mutually-exclusive. For example, it thinks the following two rules overlap:

  • Members with the async keyword are pascal case
  • Fields and locals are camelCase

Since there are no symbols which match both of these rules, it does not matter what order they appear in in the .editorconfig file. Therefore, we should not produce a warning.

I didn't fix this bug in the first pass since it will take some thought to make sure other bugs are not introduced. We can address it as a follow-up. Very few .editorconfig files will be impacted by this.

@madskristensen madskristensen merged commit d4c5f46 into madskristensen:master Jun 14, 2019
@sharwell sharwell deleted the analyze-naming-rules branch June 14, 2019 15:53
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

Successfully merging this pull request may close these issues.

2 participants