Skip to content

Commit

Permalink
linter: Report when single statement if-statement body is not on the …
Browse files Browse the repository at this point in the history
…same line as if-keyword

Fixes dart-lang/linter#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 <sra@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
  • Loading branch information
srawlins authored and Commit Queue committed Sep 24, 2024
1 parent a39ec28 commit 7d4994f
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 3 additions & 1 deletion pkg/front_end/lib/src/base/export.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion pkg/front_end/lib/src/base/incremental_compiler.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
20 changes: 15 additions & 5 deletions pkg/front_end/tool/coverage_merger.dart
Original file line number Diff line number Diff line change
Expand Up @@ -739,7 +739,9 @@ void _mergeCoverageInto(
Coverage coverage, Map<Uri, Set<int>> misses, Map<Uri, Hit> 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<int> miss = misses[fileCoverage.uri] ??= {};
miss.addAll(fileCoverage.misses);
Expand Down Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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);
}

Expand Down Expand Up @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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']);
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/linter/lib/src/rules/use_string_buffers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down

0 comments on commit 7d4994f

Please sign in to comment.