From ee660b5e1bb95c608179bacc41aa106a8ac59b0a Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Tue, 12 May 2020 18:46:27 -0700 Subject: [PATCH] [Dogfooding[ Enforce IDE0011 (Add braces) as a build warning for some IDE projects My prior attempt to enable this as a build warning in https://github.com/dotnet/roslyn/pull/44164 did not work due to #44201. This PR actually adds the build enforcement and fixes the violations. --- .editorconfig | 2 ++ .../CSharpDeclarationComputer.cs | 33 +++++++++++++++---- ...CSharpConvertLinqQueryToForEachProvider.cs | 4 ++- ...DisambiguateSameVariableCodeFixProvider.cs | 2 ++ ...IntroduceVariableService_IntroduceLocal.cs | 2 ++ ...erseForStatementCodeRefactoringProvider.cs | 2 ++ .../AbstractConflictMarkerCodeFixProvider.cs | 2 ++ .../RegularExpressions/RegexCharClass.cs | 6 ++++ .../ConflictEngine/RenamedSpansTracker.cs | 2 ++ .../AbstractSelectedMembers.cs | 2 ++ 10 files changed, 50 insertions(+), 7 deletions(-) diff --git a/.editorconfig b/.editorconfig index 699fb17642c..c21dd878f51 100644 --- a/.editorconfig +++ b/.editorconfig @@ -231,6 +231,8 @@ dotnet_diagnostic.IDE0005.severity = warning # IDE0011: Add braces csharp_prefer_braces = when_multiline:warning +# NOTE: We need the below severity entry for Add Braces due to https://github.com/dotnet/roslyn/issues/44201 +dotnet_diagnostic.IDE0011.severity = warning # IDE0035: Remove unreachable code dotnet_diagnostic.IDE0035.severity = warning diff --git a/src/Compilers/CSharp/CSharpAnalyzerDriver/CSharpDeclarationComputer.cs b/src/Compilers/CSharp/CSharpAnalyzerDriver/CSharpDeclarationComputer.cs index b930a860512..369a958180f 100644 --- a/src/Compilers/CSharp/CSharpAnalyzerDriver/CSharpDeclarationComputer.cs +++ b/src/Compilers/CSharp/CSharpAnalyzerDriver/CSharpDeclarationComputer.cs @@ -72,7 +72,11 @@ private static bool InvalidLevel(int? level) case SyntaxKind.NamespaceDeclaration: { var ns = (NamespaceDeclarationSyntax)node; - foreach (var decl in ns.Members) ComputeDeclarations(model, decl, shouldSkip, getSymbol, builder, newLevel, cancellationToken); + foreach (var decl in ns.Members) + { + ComputeDeclarations(model, decl, shouldSkip, getSymbol, builder, newLevel, cancellationToken); + } + var declInfo = GetDeclarationInfo(model, node, getSymbol, cancellationToken); builder.Add(declInfo); @@ -94,7 +98,11 @@ private static bool InvalidLevel(int? level) case SyntaxKind.InterfaceDeclaration: { var t = (TypeDeclarationSyntax)node; - foreach (var decl in t.Members) ComputeDeclarations(model, decl, shouldSkip, getSymbol, builder, newLevel, cancellationToken); + foreach (var decl in t.Members) + { + ComputeDeclarations(model, decl, shouldSkip, getSymbol, builder, newLevel, cancellationToken); + } + var attributes = GetAttributes(t.AttributeLists).Concat(GetTypeParameterListAttributes(t.TypeParameterList)); builder.Add(GetDeclarationInfo(model, node, getSymbol, attributes, cancellationToken)); return; @@ -103,7 +111,11 @@ private static bool InvalidLevel(int? level) case SyntaxKind.EnumDeclaration: { var t = (EnumDeclarationSyntax)node; - foreach (var decl in t.Members) ComputeDeclarations(model, decl, shouldSkip, getSymbol, builder, newLevel, cancellationToken); + foreach (var decl in t.Members) + { + ComputeDeclarations(model, decl, shouldSkip, getSymbol, builder, newLevel, cancellationToken); + } + var attributes = GetAttributes(t.AttributeLists); builder.Add(GetDeclarationInfo(model, node, getSymbol, attributes, cancellationToken)); return; @@ -133,7 +145,10 @@ private static bool InvalidLevel(int? level) var t = (EventDeclarationSyntax)node; if (t.AccessorList != null) { - foreach (var decl in t.AccessorList.Accessors) ComputeDeclarations(model, decl, shouldSkip, getSymbol, builder, newLevel, cancellationToken); + foreach (var decl in t.AccessorList.Accessors) + { + ComputeDeclarations(model, decl, shouldSkip, getSymbol, builder, newLevel, cancellationToken); + } } var attributes = GetAttributes(t.AttributeLists); builder.Add(GetDeclarationInfo(model, node, getSymbol, attributes, cancellationToken)); @@ -170,7 +185,10 @@ private static bool InvalidLevel(int? level) var t = (PropertyDeclarationSyntax)node; if (t.AccessorList != null) { - foreach (var decl in t.AccessorList.Accessors) ComputeDeclarations(model, decl, shouldSkip, getSymbol, builder, newLevel, cancellationToken); + foreach (var decl in t.AccessorList.Accessors) + { + ComputeDeclarations(model, decl, shouldSkip, getSymbol, builder, newLevel, cancellationToken); + } } if (t.ExpressionBody != null) @@ -259,7 +277,10 @@ private static bool InvalidLevel(int? level) case SyntaxKind.CompilationUnit: { var t = (CompilationUnitSyntax)node; - foreach (var decl in t.Members) ComputeDeclarations(model, decl, shouldSkip, getSymbol, builder, newLevel, cancellationToken); + foreach (var decl in t.Members) + { + ComputeDeclarations(model, decl, shouldSkip, getSymbol, builder, newLevel, cancellationToken); + } if (t.AttributeLists.Any()) { diff --git a/src/Features/CSharp/Portable/ConvertLinq/CSharpConvertLinqQueryToForEachProvider.cs b/src/Features/CSharp/Portable/ConvertLinq/CSharpConvertLinqQueryToForEachProvider.cs index 70843bcef93..c2c5c7e55df 100644 --- a/src/Features/CSharp/Portable/ConvertLinq/CSharpConvertLinqQueryToForEachProvider.cs +++ b/src/Features/CSharp/Portable/ConvertLinq/CSharpConvertLinqQueryToForEachProvider.cs @@ -98,8 +98,10 @@ public bool TryConvert(out DocumentUpdateInfo documentUpdateInfo) { if (!documentUpdateInfo.Source.IsParentKind(SyntaxKind.Block) && documentUpdateInfo.Destinations.Length > 1) - + { documentUpdateInfo = new DocumentUpdateInfo(documentUpdateInfo.Source, SyntaxFactory.Block(documentUpdateInfo.Destinations)); + } + return true; } diff --git a/src/Features/CSharp/Portable/DisambiguateSameVariable/CSharpDisambiguateSameVariableCodeFixProvider.cs b/src/Features/CSharp/Portable/DisambiguateSameVariable/CSharpDisambiguateSameVariableCodeFixProvider.cs index 374a18fb62e..a26952fcbfa 100644 --- a/src/Features/CSharp/Portable/DisambiguateSameVariable/CSharpDisambiguateSameVariableCodeFixProvider.cs +++ b/src/Features/CSharp/Portable/DisambiguateSameVariable/CSharpDisambiguateSameVariableCodeFixProvider.cs @@ -155,7 +155,9 @@ where m.IsAccessibleWithin(enclosingType) { if (!CanFix(semanticModel, diagnostic, cancellationToken, out var nameNode, out var matchingMember, out _)) + { continue; + } var newNameNode = matchingMember.Name.ToIdentifierName(); var newExpr = (ExpressionSyntax)newNameNode; diff --git a/src/Features/CSharp/Portable/IntroduceVariable/CSharpIntroduceVariableService_IntroduceLocal.cs b/src/Features/CSharp/Portable/IntroduceVariable/CSharpIntroduceVariableService_IntroduceLocal.cs index 5d24c1e094f..6ce0acb981c 100644 --- a/src/Features/CSharp/Portable/IntroduceVariable/CSharpIntroduceVariableService_IntroduceLocal.cs +++ b/src/Features/CSharp/Portable/IntroduceVariable/CSharpIntroduceVariableService_IntroduceLocal.cs @@ -419,8 +419,10 @@ private int GetFirstStatementAffectedIndex(SyntaxNode innermostCommonBlock, ISet var nextStatementLeading = nextStatement.GetLeadingTrivia(); var precedingEndOfLine = nextStatementLeading.LastOrDefault(t => t.Kind() == SyntaxKind.EndOfLineTrivia); if (precedingEndOfLine == default) + { return oldStatements.ReplaceRange( nextStatement, new[] { newStatement, nextStatement }); + } var endOfLineIndex = nextStatementLeading.IndexOf(precedingEndOfLine) + 1; diff --git a/src/Features/CSharp/Portable/ReverseForStatement/CSharpReverseForStatementCodeRefactoringProvider.cs b/src/Features/CSharp/Portable/ReverseForStatement/CSharpReverseForStatementCodeRefactoringProvider.cs index 20390a88919..dd06b42fb65 100644 --- a/src/Features/CSharp/Portable/ReverseForStatement/CSharpReverseForStatementCodeRefactoringProvider.cs +++ b/src/Features/CSharp/Portable/ReverseForStatement/CSharpReverseForStatementCodeRefactoringProvider.cs @@ -53,7 +53,9 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte if (declaration == null || declaration.Variables.Count != 1 || forStatement.Incrementors.Count != 1) + { return; + } var variable = declaration.Variables[0]; var after = forStatement.Incrementors[0]; diff --git a/src/Features/Core/Portable/ConflictMarkerResolution/AbstractConflictMarkerCodeFixProvider.cs b/src/Features/Core/Portable/ConflictMarkerResolution/AbstractConflictMarkerCodeFixProvider.cs index eebe4a0a83f..920e1f741ca 100644 --- a/src/Features/Core/Portable/ConflictMarkerResolution/AbstractConflictMarkerCodeFixProvider.cs +++ b/src/Features/Core/Portable/ConflictMarkerResolution/AbstractConflictMarkerCodeFixProvider.cs @@ -93,7 +93,9 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) // issue there. if (startTrivia.RawKind == _syntaxKinds.ConflictMarkerTrivia || middleTrivia.RawKind == _syntaxKinds.ConflictMarkerTrivia) + { return false; + } } return true; diff --git a/src/Features/Core/Portable/EmbeddedLanguages/RegularExpressions/RegexCharClass.cs b/src/Features/Core/Portable/EmbeddedLanguages/RegularExpressions/RegexCharClass.cs index 9b967b47b48..68532e2e93f 100644 --- a/src/Features/Core/Portable/EmbeddedLanguages/RegularExpressions/RegexCharClass.cs +++ b/src/Features/Core/Portable/EmbeddedLanguages/RegularExpressions/RegexCharClass.cs @@ -269,7 +269,9 @@ private static bool CharInClassInternal(char ch, string set, int start, int mySe // reverse this check. Debug.Assert((SETSTART & 0x1) == 1, "If SETSTART is not odd, the calculation below this will be reversed"); if ((min & 0x1) == (start & 0x1)) + { return true; + } else { if (myCategoryLength == 0) @@ -302,7 +304,9 @@ private static bool CharInCategory(char ch, string set, int start, int mySetLeng if (curcat == SpaceConst) { if (char.IsWhiteSpace(ch)) + { return true; + } else { i++; @@ -320,7 +324,9 @@ private static bool CharInCategory(char ch, string set, int start, int mySetLeng if (curcat == NotSpaceConst) { if (!char.IsWhiteSpace(ch)) + { return true; + } else { i++; diff --git a/src/Workspaces/Core/Portable/Rename/ConflictEngine/RenamedSpansTracker.cs b/src/Workspaces/Core/Portable/Rename/ConflictEngine/RenamedSpansTracker.cs index 5ab06e6c9a0..77fb7dc4592 100644 --- a/src/Workspaces/Core/Portable/Rename/ConflictEngine/RenamedSpansTracker.cs +++ b/src/Workspaces/Core/Portable/Rename/ConflictEngine/RenamedSpansTracker.cs @@ -237,8 +237,10 @@ internal async Task SimplifyAsync(Solution solution, IEnumerable>(); foreach (var (docId, spans) in _documentToComplexifiedSpansMap) + { builder.Add(docId, spans.SelectAsArray( s => new ComplexifiedSpan(s.OriginalSpan, s.NewSpan, s.ModifiedSubSpans.ToImmutableArray()))); + } return builder.ToImmutable(); } diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SelectedMembers/AbstractSelectedMembers.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SelectedMembers/AbstractSelectedMembers.cs index ccd3678475c..c9a91e86f6c 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SelectedMembers/AbstractSelectedMembers.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SelectedMembers/AbstractSelectedMembers.cs @@ -102,7 +102,9 @@ void AddSelectedFieldOrPropertyDeclarations(TMemberDeclarationSyntax member) { if (!(member is TFieldDeclarationSyntax) && !(member is TPropertyDeclarationSyntax)) + { return; + } // first, check if entire member is selected. If so, we definitely include this member. if (textSpan.Contains(member.Span)) -- GitLab