From 7d4994fa4aa5bc2657c229ada92e589166d0d163 Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Tue, 24 Sep 2024 01:59:23 +0000 Subject: [PATCH] linter: Report when single statement if-statement body is not on the same line as if-keyword Fixes https://github.com/dart-lang/linter/issues/4870 Cq-Include-Trybots: luci.dart.try:flutter-analyze-try,analyzer-win-release-try Change-Id: Ibd8969afe35f719a020e5aa37efdc1792addac7e Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/353140 Reviewed-by: Stephen Adams Reviewed-by: Phil Quitslund Reviewed-by: Johnni Winther Commit-Queue: Samuel Rawlins --- .../transformations/js_util_optimizer.dart | 4 +++- pkg/front_end/lib/src/base/export.dart | 4 +++- .../lib/src/base/incremental_compiler.dart | 4 +++- pkg/front_end/tool/coverage_merger.dart | 20 ++++++++++++++----- ...rly_braces_in_flow_control_structures.dart | 6 +++--- .../lib/src/rules/use_string_buffers.dart | 4 +++- ...races_in_flow_control_structures_test.dart | 11 ++++++++++ 7 files changed, 41 insertions(+), 12 deletions(-) diff --git a/pkg/_js_interop_checks/lib/src/transformations/js_util_optimizer.dart b/pkg/_js_interop_checks/lib/src/transformations/js_util_optimizer.dart index 7795f9a32ac5..405276a75486 100644 --- a/pkg/_js_interop_checks/lib/src/transformations/js_util_optimizer.dart +++ b/pkg/_js_interop_checks/lib/src/transformations/js_util_optimizer.dart @@ -1082,7 +1082,9 @@ class ExtensionIndex { /// If not an interop extension type or extension member, returns null. FunctionType? getFunctionType(Procedure node) { if (getExtensionDescriptor(node) == null && - getExtensionTypeDescriptor(node) == null) return null; + getExtensionTypeDescriptor(node) == null) { + return null; + } final functionType = node.signatureType ?? node.function.computeFunctionType(Nullability.nonNullable); diff --git a/pkg/front_end/lib/src/base/export.dart b/pkg/front_end/lib/src/base/export.dart index d74c1beafa4b..b75a0db71fb1 100644 --- a/pkg/front_end/lib/src/base/export.dart +++ b/pkg/front_end/lib/src/base/export.dart @@ -39,7 +39,9 @@ class Export { if (combinator.isShow && !combinator.names.contains(name)) return false; if (combinator.isHide && // Coverage-ignore(suite): Not run. - combinator.names.contains(name)) return false; + combinator.names.contains(name)) { + return false; + } } } return exporter.libraryBuilder.addToExportScope(name, member, diff --git a/pkg/front_end/lib/src/base/incremental_compiler.dart b/pkg/front_end/lib/src/base/incremental_compiler.dart index 11d5bf17d1b8..fb947a52782f 100644 --- a/pkg/front_end/lib/src/base/incremental_compiler.dart +++ b/pkg/front_end/lib/src/base/incremental_compiler.dart @@ -1765,7 +1765,9 @@ class IncrementalCompiler implements IncrementalKernelGenerator { Uri? partFileUri = uriTranslator.getPartFileUri(lib.fileUri, part); if (partsUsed != null && // Coverage-ignore(suite): Not run. - partsUsed.contains(partFileUri)) continue; + partsUsed.contains(partFileUri)) { + continue; + } // If the builders map contain the "parts" import uri, it's a real library // (erroneously) used as a part so we don't want to remove that. diff --git a/pkg/front_end/tool/coverage_merger.dart b/pkg/front_end/tool/coverage_merger.dart index 5af1be54a5ae..469c9cface90 100644 --- a/pkg/front_end/tool/coverage_merger.dart +++ b/pkg/front_end/tool/coverage_merger.dart @@ -739,7 +739,9 @@ void _mergeCoverageInto( Coverage coverage, Map> misses, Map hits) { for (FileCoverage fileCoverage in coverage.getAllFileCoverages()) { if (fileCoverage.uri.isScheme("package") && - fileCoverage.uri.pathSegments.first != "front_end") continue; + fileCoverage.uri.pathSegments.first != "front_end") { + continue; + } if (fileCoverage.misses.isNotEmpty) { Set miss = misses[fileCoverage.uri] ??= {}; miss.addAll(fileCoverage.misses); @@ -1013,7 +1015,9 @@ class AstIndexerAndIgnoreCollector extends AstIndexer { // possible "double-ignored" coverages should still work fine because of the // interval list. if (_checkCommentAndIgnoreCoverage(node.beginToken, node, - allowReplace: false)) return; + allowReplace: false)) { + return; + } super.visitClassDeclarationEnd(node); } @@ -1024,7 +1028,9 @@ class AstIndexerAndIgnoreCollector extends AstIndexer { // possible "double-ignored" coverages should still work fine because of the // interval list. if (_checkCommentAndIgnoreCoverage(node.beginToken, node, - allowReplace: false)) return; + allowReplace: false)) { + return; + } super.visitExtensionDeclarationEnd(node); } @@ -1035,7 +1041,9 @@ class AstIndexerAndIgnoreCollector extends AstIndexer { // possible "double-ignored" coverages should still work fine because of the // interval list. if (_checkCommentAndIgnoreCoverage(node.beginToken, node, - allowReplace: false)) return; + allowReplace: false)) { + return; + } super.visitExtensionTypeDeclarationEnd(node); } @@ -1064,7 +1072,9 @@ class AstIndexerAndIgnoreCollector extends AstIndexer { @override void visitTopLevelMethodEnd(TopLevelMethodEnd node) { if (_checkCommentAndIgnoreCoverage(node.beginToken, node, - allowReplace: false)) return; + allowReplace: false)) { + return; + } super.visitTopLevelMethodEnd(node); String name = node.getNameIdentifier().token.lexeme; if (topLevelMethodNamesToIgnore.contains(name)) { diff --git a/pkg/linter/lib/src/rules/curly_braces_in_flow_control_structures.dart b/pkg/linter/lib/src/rules/curly_braces_in_flow_control_structures.dart index 8f2300278b57..df4201cce2bb 100644 --- a/pkg/linter/lib/src/rules/curly_braces_in_flow_control_structures.dart +++ b/pkg/linter/lib/src/rules/curly_braces_in_flow_control_structures.dart @@ -32,8 +32,8 @@ if (isWeekDay) { ``` There is one exception to this: an `if` statement with no `else` clause where -the entire `if` statement and the then body all fit in one line. In that case, -you may leave off the braces if you prefer: +the entire `if` statement (including the condition and the body) fits in one +line. In that case, you may leave off the braces if you prefer: **GOOD:** ```dart @@ -105,7 +105,7 @@ class _Visitor extends SimpleAstVisitor { var unit = node.root as CompilationUnit; var lineInfo = unit.lineInfo; - if (lineInfo.getLocation(node.rightParenthesis.end).lineNumber != + if (lineInfo.getLocation(node.ifKeyword.offset).lineNumber != lineInfo.getLocation(node.thenStatement.end).lineNumber) { rule.reportLint(node.thenStatement, arguments: ['an if']); } diff --git a/pkg/linter/lib/src/rules/use_string_buffers.dart b/pkg/linter/lib/src/rules/use_string_buffers.dart index 24945033f896..79603a222d92 100644 --- a/pkg/linter/lib/src/rules/use_string_buffers.dart +++ b/pkg/linter/lib/src/rules/use_string_buffers.dart @@ -122,7 +122,9 @@ class _UseStringBufferVisitor extends SimpleAstVisitor { @override void visitAssignmentExpression(AssignmentExpression node) { if (node.operator.type != TokenType.PLUS_EQ && - node.operator.type != TokenType.EQ) return; + node.operator.type != TokenType.EQ) { + return; + } var left = node.leftHandSide; var writeType = node.writeType; diff --git a/pkg/linter/test/rules/curly_braces_in_flow_control_structures_test.dart b/pkg/linter/test/rules/curly_braces_in_flow_control_structures_test.dart index afafd358c737..3cf19270f2b7 100644 --- a/pkg/linter/test/rules/curly_braces_in_flow_control_structures_test.dart +++ b/pkg/linter/test/rules/curly_braces_in_flow_control_structures_test.dart @@ -165,6 +165,17 @@ void f() { '''); } + test_ifStatement_singleStatement_sameLine_multiLineCondition() async { + await assertDiagnostics(r''' +void f() { + if (1 == + 2) return; +} +''', [ + lint(31, 7), + ]); + } + test_ifStatementElse_block_sameLine() async { await assertNoDiagnostics(r''' void f() {