Update SA1206 to handle c# 11 modifier "required" #3535

bjornhellander · 2022-07-17T09:19:29Z

codecov · 2022-07-17T09:53:44Z

Codecov Report

Merging #3535 (987eb30) into master (84e2324) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 987eb30 differs from pull request most recent head e40ab1f. Consider uploading reports for the commit e40ab1f to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3535      +/-   ##
==========================================
- Coverage   93.25%   93.25%   -0.01%     
==========================================
  Files        1078     1078              
  Lines      113689   113709      +20     
  Branches     4020     4020              
==========================================
+ Hits       106024   106041      +17     
- Misses       6630     6633       +3     
  Partials     1035     1035

bjornhellander · 2023-02-11T13:59:23Z

Rebased and added the missing tests, @sharwell

KUTlime · 2023-02-13T17:02:53Z

@bjornhellander @sharwell How is this PR going? I have to write #pragmas more than I wish to. 🤷‍♀️

sharwell · 2023-02-17T20:20:47Z

StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1206DeclarationKeywordsMustFollowOrder.cs

+                if (currentModifierType == ModifierType.None)
+                {
+                    // Just ignore unknown modifiers, to avoid false positives before support for
+                    // new language features has been added.
+                    continue;
+                }


💭 I almost prefer the original behavior, otherwise support may never have been added.

I think that "never" is unlikely, but it would almost certainly have taken longer to be discovered. I was just trying to think about the user experience and for my part I would definitely prefer false negatives over false positives, but I can revert that change if you want me to. Just say the word.

@sharwell Could you please make you call here? Thank you.

Ok. I reverted this change now.

Imo behavior proposed by @bjornhellander is much better. We have required keyword for >6 months. This means users of stylecop either do not use SA1206 or do not use required keyword over this period.

@POnakS We do use it (required keyword) but we have to constantly disable SA1206 by pragma.

@KUTlime That's a mess, I've decided to just disable SA1206, as potential risk to my codebase is small, and having pragma everywhere is bigger concern IMO (you need to clean this up manually, and if you don't you won't have inspection)

DotNetAnalyzers#3527

bjornhellander · 2023-03-26T11:35:31Z

Anything else, @sharwell?

KUTlime · 2023-04-11T13:29:51Z

Ground control to @sharwell, are you alive? @bjornhellander requested a feedback. We are waiting for this.

sharwell · 2023-04-11T14:50:49Z

Getting hammered recently. Team is running on a light staff after some organizational changes.

KUTlime · 2023-04-11T14:58:00Z

Getting hammered recently. Team is running on a light staff after some organizational changes.

I recommend soft drink next time. They are call soft for a reason. 😉 #Sarcasm

bjornhellander · 2023-04-11T18:30:49Z

Thanks for the merge! 👍

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [StyleCop.Analyzers](https://togithub.com/DotNetAnalyzers/StyleCopAnalyzers) | nuget | patch | `1.2.0-beta.435` -> `1.2.0-beta.507` | --- ### Release Notes <details> <summary>DotNetAnalyzers/StyleCopAnalyzers</summary> ### [`v1.2.0-beta.507`](https://togithub.com/DotNetAnalyzers/StyleCopAnalyzers/releases/tag/1.2.0-beta.507) [Compare Source](https://togithub.com/DotNetAnalyzers/StyleCopAnalyzers/compare/1.2.0-beta.435...1.2.0-beta.507) #### What's Changed - Update to StyleCop.Analyzers 1.2.0-beta.435 by [@sharwell](https://togithub.com/sharwell) in [DotNetAnalyzers/StyleCopAnalyzers#3499 - Add c# 11 test project to opencover-report.ps1 by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3506 - Use GetText instead of ToFullString by [@sharwell](https://togithub.com/sharwell) in [DotNetAnalyzers/StyleCopAnalyzers#3514 - Keep tracked nodes in a list by [@sharwell](https://togithub.com/sharwell) in [DotNetAnalyzers/StyleCopAnalyzers#3525 - Remove unnecessary nullable directives by [@sharwell](https://togithub.com/sharwell) in [DotNetAnalyzers/StyleCopAnalyzers#3530 - Remove hard-coded language versions in test projects for c# 8, 9 and 10 by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3528 - Update SA1515 to not let one range of trivia affect another by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3529 - Mentioned VS 2022 by [@twojnarowski](https://togithub.com/twojnarowski) in [DotNetAnalyzers/StyleCopAnalyzers#3549 - Remove byte order mark from schema file by [@martincostello](https://togithub.com/martincostello) in [DotNetAnalyzers/StyleCopAnalyzers#3562 - Update SA1012 to expect no space between a property pattern's opening brace and an enclosing list pattern's opening bracket by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3511 - Update Microsoft.CodeAnalysis.CSharp.Workspaces to version 4.4.0 for the c# 11 test project by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3580 - Update SA1008 to handle positional patterns inside property patterns by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3579 - Update SA1000 to trigger after keywords is, or, and, not by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3585 - Update SA1000.md by [@Youssef1313](https://togithub.com/Youssef1313) in [DotNetAnalyzers/StyleCopAnalyzers#3563 - Update SA1313 to also allow incorrect names in explicitly implemented methods from interfaces by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3569 - Update SA1023 to not trigger first in line, inside a foreach without braces by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3543 - Update SA1400 to recognize access modifier "file" by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3590 - Update SA1206 to recognize modifier "file" by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3591 - Update SA1000 to handle checked operator declarations correctly by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3505 - Update SA1402 to handle records and record structs by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3570 - Bump Newtonsoft.Json from 12.0.3 to 13.0.2 in /StyleCop.Analyzers/StyleCop.Analyzers.Status.Generator by [@dependabot](https://togithub.com/dependabot) in [DotNetAnalyzers/StyleCopAnalyzers#3584 - Update to the latest version of the testing library by [@sharwell](https://togithub.com/sharwell) in [DotNetAnalyzers/StyleCopAnalyzers#3601 - Update so that SA1600 tests will be run with the expected language version in test projects for c# 8 and above by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3614 - Update reading of file_header_template and stylecop.documentation.copyrightText to allow multiple lines by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3617 - Update SA1015 to require trailing space after an explicit generic return type in a lambda expression by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3625 - Update to Microsoft.CodeAnalysis.Analyzers 3.3.5-beta1.23205.2 by [@sharwell](https://togithub.com/sharwell) in [DotNetAnalyzers/StyleCopAnalyzers#3628 - Update SA1206 to handle c# 11 modifier "required" by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3535 - Preparations for SettingsHelper optimizations by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3635 - Correct SA1515 to not fire on the second line of a file header by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3633 - Update AnalyzersExtensions and SettingsHelper to use cached JsonValue objects where possible by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3642 - Update SA1010 to not trigger on list patterns by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3507 - Update NamingSettings and DocumentationSettings to keep one Regex instance instead of calling Regex.IsMatch by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3639 - Use ResxSourceGenerator for resource generation by [@sharwell](https://togithub.com/sharwell) in [DotNetAnalyzers/StyleCopAnalyzers#3343 - Make XmlCommentHelper faster by [@ninedan](https://togithub.com/ninedan) in [DotNetAnalyzers/StyleCopAnalyzers#3651 - Update RenameToUpperCaseCodeFixProvider to not offer a code fix if the identifier only consists of underscores by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3637 - Don't emit SA1414 for interface implementations by [@CollinAlpert](https://togithub.com/CollinAlpert) in [DotNetAnalyzers/StyleCopAnalyzers#3644 - Support file-scoped namespaces in SA1516 by [@JakubLinhart](https://togithub.com/JakubLinhart) in [DotNetAnalyzers/StyleCopAnalyzers#3513 - Update SA1137 to also consider init accessors by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3669 - Update SA1500 to also consider init accessors by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3670 - Update SA1513 to not trigger before an init accessor by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3666 - Update SA1212 to also trigger for an init accessor before a getter by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3661 - Update SA1513 codefix to use the existing newline character sequence by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3607 - Correct code fix for SA1130 when delegate expression is part of a cast expression by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3516 - Update so that c# 7 tests will be run with the expected language version in test projects for c# 8 and above by [@bjornhellander](https://togithub.com/bjornhellander) in [DotNetAnalyzers/StyleCopAnalyzers#3616 - SA1629 should allow full-sentence links instead of forcing the period to glow white by [@jnm2](https://togithub.com/jnm2) in [DotNetAnalyzers/StyleCopAnalyzers#3371 #### New Contributors - [@twojnarowski](https://togithub.com/twojnarowski) made their first contribution in [DotNetAnalyzers/StyleCopAnalyzers#3549 - [@ninedan](https://togithub.com/ninedan) made their first contribution in [DotNetAnalyzers/StyleCopAnalyzers#3651 - [@CollinAlpert](https://togithub.com/CollinAlpert) made their first contribution in [DotNetAnalyzers/StyleCopAnalyzers#3644 - [@JakubLinhart](https://togithub.com/JakubLinhart) made their first contribution in [DotNetAnalyzers/StyleCopAnalyzers#3513 **Full Changelog**: DotNetAnalyzers/StyleCopAnalyzers@1.2.0-beta.435...1.2.0-beta.507 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "before 4am" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/ThorstenSauter/NoPlan).  --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Thorsten Sauter <Thorsten.Sauter@gmail.com>

bjornhellander mentioned this pull request Jul 28, 2022

Incorrect formatting behaviour when using SyntaxFactory.Block dotnet/roslyn#63013

Open

bjornhellander force-pushed the feature/sa1206-required branch from 75ce91d to a15c944 Compare August 10, 2022 06:37

bjornhellander mentioned this pull request Nov 27, 2022

Update Microsoft.CodeAnalysis.CSharp.Workspaces to version 4.4.0 for the c# 11 test project #3580

Merged

bjornhellander force-pushed the feature/sa1206-required branch 2 times, most recently from ec4568a to 259e3dd Compare February 11, 2023 13:12

bjornhellander changed the title ~~Update SA1206 to handle c# 11 modifier "required" [work in progress]~~ Update SA1206 to handle c# 11 modifier "required" Feb 11, 2023

sharwell reviewed Feb 17, 2023

View reviewed changes

bjornhellander force-pushed the feature/sa1206-required branch from 259e3dd to 6b80473 Compare March 26, 2023 08:14

Update SA1206 to handle c# 11 modifier "required"

e40ab1f

DotNetAnalyzers#3527

bjornhellander force-pushed the feature/sa1206-required branch from 6b80473 to e40ab1f Compare March 26, 2023 09:35

sharwell approved these changes Apr 11, 2023

View reviewed changes

sharwell merged commit d02bf2b into DotNetAnalyzers:master Apr 11, 2023
4 checks passed

sharwell added this to the 1.2-beta.next milestone Apr 11, 2023

bjornhellander deleted the feature/sa1206-required branch April 11, 2023 18:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update SA1206 to handle c# 11 modifier "required" #3535

Update SA1206 to handle c# 11 modifier "required" #3535

bjornhellander commented Jul 17, 2022 •

edited

codecov bot commented Jul 17, 2022 •

edited

bjornhellander commented Feb 11, 2023

KUTlime commented Feb 13, 2023

sharwell Feb 17, 2023

bjornhellander Feb 19, 2023

KUTlime Mar 10, 2023

bjornhellander Mar 26, 2023

POnakS Mar 26, 2023

KUTlime Mar 28, 2023

POnakS Mar 28, 2023

bjornhellander commented Mar 26, 2023

KUTlime commented Apr 11, 2023

sharwell commented Apr 11, 2023

KUTlime commented Apr 11, 2023

bjornhellander commented Apr 11, 2023

Update SA1206 to handle c# 11 modifier "required" #3535

Update SA1206 to handle c# 11 modifier "required" #3535

Conversation

bjornhellander commented Jul 17, 2022 • edited

codecov bot commented Jul 17, 2022 • edited

Codecov Report

bjornhellander commented Feb 11, 2023

KUTlime commented Feb 13, 2023

sharwell Feb 17, 2023

Choose a reason for hiding this comment

bjornhellander Feb 19, 2023

Choose a reason for hiding this comment

KUTlime Mar 10, 2023

Choose a reason for hiding this comment

bjornhellander Mar 26, 2023

Choose a reason for hiding this comment

POnakS Mar 26, 2023

Choose a reason for hiding this comment

KUTlime Mar 28, 2023

Choose a reason for hiding this comment

POnakS Mar 28, 2023

Choose a reason for hiding this comment

bjornhellander commented Mar 26, 2023

KUTlime commented Apr 11, 2023

sharwell commented Apr 11, 2023

KUTlime commented Apr 11, 2023

bjornhellander commented Apr 11, 2023

bjornhellander commented Jul 17, 2022 •

edited

codecov bot commented Jul 17, 2022 •

edited