From 8de7b87ed5541fd710b0af67ae6bde7c0f1577ec Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Thu, 10 Aug 2017 19:24:53 +0100 Subject: [PATCH 01/11] Tweak comment that was based on integration test verison. See #30285 --- .../test/src/computer/closingLabels_computer_test.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart b/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart index 17ca20587349..c33d9afd8725 100644 --- a/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart +++ b/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart @@ -284,8 +284,8 @@ Widget build(BuildContext context) { return computer.compute(); } - /// Compares the latest received closing labels with expected - /// labels extracted from the comments in the test file. + /// Compares provided closing labels with expected + /// labels extracted from the comments in the provided content. _compareLabels(List labels, String content, {int expectedLabelCount}) { // Require the test pass us the expected count to guard From ed036a2510cf17be9e310cece3e572ed9f02d506 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Thu, 10 Aug 2017 20:21:55 +0100 Subject: [PATCH 02/11] Include additional 1/2-line spans that end on same lines as >2 line spans. See #30285 --- .../src/computer/computer_closingLabels.dart | 51 +++++++++++-------- .../computer/closingLabels_computer_test.dart | 20 ++++++++ 2 files changed, 49 insertions(+), 22 deletions(-) diff --git a/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart b/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart index 1ff462ba4708..e764322e962d 100644 --- a/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart +++ b/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart @@ -7,13 +7,18 @@ import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; import 'package:analyzer/src/generated/source.dart'; +class _ClosingLabelWithLineCount { + ClosingLabel label; + int spannedLines; +} + /** * A computer for [CompilationUnit] closing labels. */ class DartUnitClosingLabelsComputer { final LineInfo _lineInfo; final CompilationUnit _unit; - final List _closingLabels = []; + final Map> _closingLabelsByEndLine = {}; DartUnitClosingLabelsComputer(this._lineInfo, this._unit); @@ -22,7 +27,11 @@ class DartUnitClosingLabelsComputer { */ List compute() { _unit.accept(new _DartUnitClosingLabelsComputerVisitor(this)); - return _closingLabels; + return _closingLabelsByEndLine.values + .where((l) => l.any((cl) => cl.spannedLines >= 2)) + .expand((cls) => cls) + .map((clwlc) => clwlc.label) + .toList(); } } @@ -37,19 +46,17 @@ class _DartUnitClosingLabelsComputerVisitor @override Object visitInstanceCreationExpression(InstanceCreationExpression node) { - if (_spansManyLines(node)) { - var label = node.constructorName.type.name.name; - if (node.constructorName.name != null) - label += ".${node.constructorName.name.name}"; - _addLabel(node, label); - } + var label = node.constructorName.type.name.name; + if (node.constructorName.name != null) + label += ".${node.constructorName.name.name}"; + _addLabel(node, label); return super.visitInstanceCreationExpression(node); } @override Object visitMethodInvocation(MethodInvocation node) { - if (node.argumentList != null && _spansManyLines(node)) { + if (node.argumentList != null) { final target = node.target; final label = target is Identifier ? "${target.name}.${node.methodName.name}" @@ -62,27 +69,27 @@ class _DartUnitClosingLabelsComputerVisitor @override visitListLiteral(ListLiteral node) { - if (_spansManyLines(node)) { - final args = node.typeArguments?.arguments; - final typeName = args != null ? args[0]?.toString() : null; + final args = node.typeArguments?.arguments; + final typeName = args != null ? args[0]?.toString() : null; - if (typeName != null) { - _addLabel(node, "List<$typeName>"); - } + if (typeName != null) { + _addLabel(node, "List<$typeName>"); } return super.visitListLiteral(node); } - bool _spansManyLines(AstNode node) { + void _addLabel(AstNode node, String label) { final start = computer._lineInfo.getLocation(node.offset); final end = computer._lineInfo.getLocation(node.end - 1); + final closingLabel = new ClosingLabel(node.offset, node.length, label); + final labelWithSpan = new _ClosingLabelWithLineCount() + ..label = closingLabel + ..spannedLines = end.lineNumber - start.lineNumber; - return end.lineNumber - start.lineNumber > 1; - } - - void _addLabel(AstNode node, String label) { - computer._closingLabels - .add(new ClosingLabel(node.offset, node.length, label)); + if (!computer._closingLabelsByEndLine.containsKey(end.lineNumber)) { + computer._closingLabelsByEndLine[end.lineNumber] = []; + } + computer._closingLabelsByEndLine[end.lineNumber].add(labelWithSpan); } } diff --git a/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart b/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart index c33d9afd8725..a9248508d4bb 100644 --- a/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart +++ b/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart @@ -247,6 +247,26 @@ void myMethod() { _compareLabels(labels, content, expectedLabelCount: 2); } + /// When a line contains the end of a label, we need to ensure we also include any + /// other labels that end on the same line, even if they are 1-2 lines, otherwise + /// it isn't obvious which closing bracket goes with the label. + test_mixedLineSpanning() async { + String content = """ +main() { + /*1*/expectedLabels.forEach((m) { + /*2*/expect( + labels, + /*3*/contains( + /*4*/new ClosingLabel(expectedStart, expectedLength, expectedLabel)/*4:ClosingLabel*/)/*3:contains*/)/*2:expect*/; + })/*1:expectedLabels.forEach*/; + } +} + """; + + var labels = await _computeElements(content); + _compareLabels(labels, content, expectedLabelCount: 4); + } + test_knownBadCode1() async { // This code crashed during testing when I accidentally inserted a test snippet. String content = """ From 7e4b22fbce84fe8e9ee946e5cf8a1a074c246d23 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Thu, 10 Aug 2017 20:40:34 +0100 Subject: [PATCH 03/11] Add tests to ensure no unwanted closing labels from multiline expressions. See #30285 --- .../src/computer/computer_closingLabels.dart | 26 +++++++++----- .../computer/closingLabels_computer_test.dart | 35 +++++++++++++++++++ 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart b/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart index e764322e962d..aa6f9aca768b 100644 --- a/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart +++ b/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart @@ -46,10 +46,16 @@ class _DartUnitClosingLabelsComputerVisitor @override Object visitInstanceCreationExpression(InstanceCreationExpression node) { - var label = node.constructorName.type.name.name; - if (node.constructorName.name != null) - label += ".${node.constructorName.name.name}"; - _addLabel(node, label); + if (node.argumentList != null) { + var label = node.constructorName.type.name.name; + if (node.constructorName.name != null) + label += ".${node.constructorName.name.name}"; + // We override the node used for doing line calculations because otherwise constructors + // that split over multiple lines (but have parens on same line) would incorrectly + // get labels, because node.start on an instance creation expression starts at the start + // of the expression. + _addLabel(node, label, checkLinesUsing: node.argumentList); + } return super.visitInstanceCreationExpression(node); } @@ -61,7 +67,10 @@ class _DartUnitClosingLabelsComputerVisitor final label = target is Identifier ? "${target.name}.${node.methodName.name}" : node.methodName.name; - _addLabel(node, label); + // We override the node used for doing line calculations because otherwise methods + // that chain over multiple lines (but have parens on same line) would incorrectly + // get labels, because node.start on a methodInvocation starts at the start of the expression. + _addLabel(node, label, checkLinesUsing: node.argumentList); } return super.visitMethodInvocation(node); @@ -79,9 +88,10 @@ class _DartUnitClosingLabelsComputerVisitor return super.visitListLiteral(node); } - void _addLabel(AstNode node, String label) { - final start = computer._lineInfo.getLocation(node.offset); - final end = computer._lineInfo.getLocation(node.end - 1); + void _addLabel(AstNode node, String label, {AstNode checkLinesUsing}) { + checkLinesUsing = checkLinesUsing ?? node; + final start = computer._lineInfo.getLocation(checkLinesUsing.offset); + final end = computer._lineInfo.getLocation(checkLinesUsing.end - 1); final closingLabel = new ClosingLabel(node.offset, node.length, label); final labelWithSpan = new _ClosingLabelWithLineCount() ..label = closingLabel diff --git a/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart b/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart index a9248508d4bb..0684f6a74276 100644 --- a/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart +++ b/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart @@ -267,6 +267,41 @@ main() { _compareLabels(labels, content, expectedLabelCount: 4); } + /// When chaining methods like this, the node's start position is on the first line + /// of the expression and not where the opening paren is, so this test ensures we + /// dont end up with lots of unwanted labels on each line here. + test_chainedMethodsOverManyLines() async { + String content = """ +List compute() { + _unit.accept(new _DartUnitClosingLabelsComputerVisitor(this)); + return _closingLabelsByEndLine.values + .where((l) => l.any((cl) => cl.spannedLines >= 2)) + .expand((cls) => cls) + .map((clwlc) => clwlc.label) + .toList(); +} + """; + + var labels = await _computeElements(content); + _compareLabels(labels, content, expectedLabelCount: 0); + } + + /// When constructors span many like this, the node's start position is on the first line + /// of the expression and not where the opening paren is, so this test ensures we + /// dont end up with lots of unwanted labels on each line here. + test_chainedConstructorOverManyLines() async { + String content = """ +main() { + return new thing + .whatIsSplit + .acrossManyLines(1, 2); +} + """; + + var labels = await _computeElements(content); + _compareLabels(labels, content, expectedLabelCount: 0); + } + test_knownBadCode1() async { // This code crashed during testing when I accidentally inserted a test snippet. String content = """ From b17f92c1cc0f4ce6489d70c1318ad4f3956d53a2 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Fri, 11 Aug 2017 10:22:02 +0100 Subject: [PATCH 04/11] Don't show closing labels for code inside interpolated strings. See #30285 --- .../lib/src/computer/computer_closingLabels.dart | 16 ++++++++++++++++ .../computer/closingLabels_computer_test.dart | 14 ++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart b/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart index aa6f9aca768b..5d00d3c384bb 100644 --- a/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart +++ b/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart @@ -88,7 +88,23 @@ class _DartUnitClosingLabelsComputerVisitor return super.visitListLiteral(node); } + int interpolatedStringsEntered = 0; + @override + visitStringInterpolation(StringInterpolation node) { + interpolatedStringsEntered++; + try { + return super.visitStringInterpolation(node); + } finally { + interpolatedStringsEntered--; + } + } + void _addLabel(AstNode node, String label, {AstNode checkLinesUsing}) { + // Never add labels if we're inside strings. + if (interpolatedStringsEntered > 0) { + return; + } + checkLinesUsing = checkLinesUsing ?? node; final start = computer._lineInfo.getLocation(checkLinesUsing.offset); final end = computer._lineInfo.getLocation(checkLinesUsing.end - 1); diff --git a/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart b/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart index 0684f6a74276..b130ed1b8e9c 100644 --- a/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart +++ b/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart @@ -302,6 +302,20 @@ main() { _compareLabels(labels, content, expectedLabelCount: 0); } + @soloTest + test_NoLabelsFromInterpolatedStrings() async { + String content = """ +void main(HighlightRegionType type, int offset, int length) { + /*1*/fail( + 'Not expected to find (offset=\$offset; length=\$length; type=\$type) in\\n' + '\${regions.join('\\n')}')/*1:fail*/; +} + """; + + var labels = await _computeElements(content); + _compareLabels(labels, content, expectedLabelCount: 1); + } + test_knownBadCode1() async { // This code crashed during testing when I accidentally inserted a test snippet. String content = """ From 7c2ce96da3b0f1448b5ce123a676c8c6b0718e9b Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Fri, 11 Aug 2017 10:37:03 +0100 Subject: [PATCH 05/11] Remove unwanted @soloTest. See #30285 --- .../test/src/computer/closingLabels_computer_test.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart b/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart index b130ed1b8e9c..9e811cf11801 100644 --- a/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart +++ b/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart @@ -302,7 +302,6 @@ main() { _compareLabels(labels, content, expectedLabelCount: 0); } - @soloTest test_NoLabelsFromInterpolatedStrings() async { String content = """ void main(HighlightRegionType type, int offset, int length) { From 21b47791945ab2ceb624e862c33f94455add9810 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Fri, 11 Aug 2017 10:38:31 +0100 Subject: [PATCH 06/11] Make casing of test names consistent. See #30285 --- .../test/src/computer/closingLabels_computer_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart b/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart index 9e811cf11801..07dfdc9fea9e 100644 --- a/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart +++ b/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart @@ -302,7 +302,7 @@ main() { _compareLabels(labels, content, expectedLabelCount: 0); } - test_NoLabelsFromInterpolatedStrings() async { + test_NnLabelsFromInterpolatedStrings() async { String content = """ void main(HighlightRegionType type, int offset, int length) { /*1*/fail( From dbbdc6608ceba6986357cfb12f96139192c2f76d Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Fri, 11 Aug 2017 15:20:46 +0100 Subject: [PATCH 07/11] Add a constructor and make fields final. See #30285 --- .../lib/src/computer/computer_closingLabels.dart | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart b/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart index 5d00d3c384bb..ddba89511a5b 100644 --- a/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart +++ b/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart @@ -8,8 +8,12 @@ import 'package:analyzer/dart/ast/visitor.dart'; import 'package:analyzer/src/generated/source.dart'; class _ClosingLabelWithLineCount { - ClosingLabel label; - int spannedLines; + final ClosingLabel label; + final int spannedLines; + + _ClosingLabelWithLineCount(ClosingLabel label, int spannedLines) + : this.label = label, + this.spannedLines = spannedLines; } /** @@ -109,9 +113,8 @@ class _DartUnitClosingLabelsComputerVisitor final start = computer._lineInfo.getLocation(checkLinesUsing.offset); final end = computer._lineInfo.getLocation(checkLinesUsing.end - 1); final closingLabel = new ClosingLabel(node.offset, node.length, label); - final labelWithSpan = new _ClosingLabelWithLineCount() - ..label = closingLabel - ..spannedLines = end.lineNumber - start.lineNumber; + final labelWithSpan = new _ClosingLabelWithLineCount( + closingLabel, end.lineNumber - start.lineNumber); if (!computer._closingLabelsByEndLine.containsKey(end.lineNumber)) { computer._closingLabelsByEndLine[end.lineNumber] = []; From 6aa86f8465c1d6197209553d69b6be818d34857a Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Fri, 11 Aug 2017 15:22:40 +0100 Subject: [PATCH 08/11] Use putIfAbsent for adding to map. See #30285 --- .../lib/src/computer/computer_closingLabels.dart | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart b/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart index ddba89511a5b..6f0a9fe05a15 100644 --- a/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart +++ b/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart @@ -116,9 +116,8 @@ class _DartUnitClosingLabelsComputerVisitor final labelWithSpan = new _ClosingLabelWithLineCount( closingLabel, end.lineNumber - start.lineNumber); - if (!computer._closingLabelsByEndLine.containsKey(end.lineNumber)) { - computer._closingLabelsByEndLine[end.lineNumber] = []; - } - computer._closingLabelsByEndLine[end.lineNumber].add(labelWithSpan); + computer._closingLabelsByEndLine + .putIfAbsent(end.lineNumber, () => <_ClosingLabelWithLineCount>[]) + .add(labelWithSpan); } } From aa2f348f77bf9212d79bf2069cc1313e7b81ac18 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Fri, 11 Aug 2017 17:30:00 +0100 Subject: [PATCH 09/11] Shorten closing labels constructor. See #30285 --- .../lib/src/computer/computer_closingLabels.dart | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart b/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart index 6f0a9fe05a15..a1b9e10c9094 100644 --- a/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart +++ b/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart @@ -11,9 +11,7 @@ class _ClosingLabelWithLineCount { final ClosingLabel label; final int spannedLines; - _ClosingLabelWithLineCount(ClosingLabel label, int spannedLines) - : this.label = label, - this.spannedLines = spannedLines; + _ClosingLabelWithLineCount(ClosingLabel this.label, int this.spannedLines); } /** From 77d8716e3bac258636f6c49584b11c48a006f67f Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Fri, 11 Aug 2017 17:30:58 +0100 Subject: [PATCH 10/11] Run "Sort members" on closing labels files. See #30285 --- .../src/computer/computer_closingLabels.dart | 41 +-- .../notification_closingLabels_test.dart | 50 +-- .../computer/closingLabels_computer_test.dart | 318 +++++++++--------- 3 files changed, 205 insertions(+), 204 deletions(-) diff --git a/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart b/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart index a1b9e10c9094..830cdda20db1 100644 --- a/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart +++ b/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart @@ -7,13 +7,6 @@ import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; import 'package:analyzer/src/generated/source.dart'; -class _ClosingLabelWithLineCount { - final ClosingLabel label; - final int spannedLines; - - _ClosingLabelWithLineCount(ClosingLabel this.label, int this.spannedLines); -} - /** * A computer for [CompilationUnit] closing labels. */ @@ -37,6 +30,13 @@ class DartUnitClosingLabelsComputer { } } +class _ClosingLabelWithLineCount { + final ClosingLabel label; + final int spannedLines; + + _ClosingLabelWithLineCount(ClosingLabel this.label, int this.spannedLines); +} + /** * An AST visitor for [DartUnitClosingLabelsComputer]. */ @@ -44,6 +44,8 @@ class _DartUnitClosingLabelsComputerVisitor extends RecursiveAstVisitor { final DartUnitClosingLabelsComputer computer; + int interpolatedStringsEntered = 0; + _DartUnitClosingLabelsComputerVisitor(this.computer); @override @@ -62,6 +64,18 @@ class _DartUnitClosingLabelsComputerVisitor return super.visitInstanceCreationExpression(node); } + @override + visitListLiteral(ListLiteral node) { + final args = node.typeArguments?.arguments; + final typeName = args != null ? args[0]?.toString() : null; + + if (typeName != null) { + _addLabel(node, "List<$typeName>"); + } + + return super.visitListLiteral(node); + } + @override Object visitMethodInvocation(MethodInvocation node) { if (node.argumentList != null) { @@ -78,19 +92,6 @@ class _DartUnitClosingLabelsComputerVisitor return super.visitMethodInvocation(node); } - @override - visitListLiteral(ListLiteral node) { - final args = node.typeArguments?.arguments; - final typeName = args != null ? args[0]?.toString() : null; - - if (typeName != null) { - _addLabel(node, "List<$typeName>"); - } - - return super.visitListLiteral(node); - } - - int interpolatedStringsEntered = 0; @override visitStringInterpolation(StringInterpolation node) { interpolatedStringsEntered++; diff --git a/pkg/analysis_server/test/analysis/notification_closingLabels_test.dart b/pkg/analysis_server/test/analysis/notification_closingLabels_test.dart index 076e4d10b59d..7ddcfc1d6efc 100644 --- a/pkg/analysis_server/test/analysis/notification_closingLabels_test.dart +++ b/pkg/analysis_server/test/analysis/notification_closingLabels_test.dart @@ -20,19 +20,25 @@ main() { @reflectiveTest class _AnalysisNotificationClosingLabelsTest extends AbstractAnalysisTest { - List lastLabels; + static const sampleCode = ''' +Widget build(BuildContext context) { + return /*1*/new Row( + children: /*2*/[ + new Text('a'), + new Text('b'), + ]/*/2*/, + )/*/1*/; +} +'''; - Completer _labelsReceived; + static final expectedResults = [ + new ClosingLabel(51, 96, "Row"), + new ClosingLabel(79, 57, "List") + ]; - void subscribeForLabels() { - addAnalysisSubscription(AnalysisService.CLOSING_LABELS, testFile); - } + List lastLabels; - Future waitForLabels(action()) { - _labelsReceived = new Completer(); - action(); - return _labelsReceived.future; - } + Completer _labelsReceived; void processNotification(Notification notification) { if (notification.event == ANALYSIS_NOTIFICATION_CLOSING_LABELS) { @@ -54,21 +60,9 @@ class _AnalysisNotificationClosingLabelsTest extends AbstractAnalysisTest { createProject(); } - static const sampleCode = ''' -Widget build(BuildContext context) { - return /*1*/new Row( - children: /*2*/[ - new Text('a'), - new Text('b'), - ]/*/2*/, - )/*/1*/; -} -'''; - - static final expectedResults = [ - new ClosingLabel(51, 96, "Row"), - new ClosingLabel(79, 57, "List") - ]; + void subscribeForLabels() { + addAnalysisSubscription(AnalysisService.CLOSING_LABELS, testFile); + } test_afterAnalysis() async { addTestFile(sampleCode); @@ -98,4 +92,10 @@ Widget build(BuildContext context) { expect(lastLabels, expectedResults); } + + Future waitForLabels(action()) { + _labelsReceived = new Completer(); + action(); + return _labelsReceived.future; + } } diff --git a/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart b/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart index 07dfdc9fea9e..f0247d1752c9 100644 --- a/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart +++ b/pkg/analysis_server/test/src/computer/closingLabels_computer_test.dart @@ -27,56 +27,51 @@ class ClosingLabelsComputerTest extends AbstractContextTest { sourcePath = provider.convertPath('/p/lib/source.dart'); } - test_multipleNested() async { + test_adjacentLinesExcluded() async { String content = """ -Widget build(BuildContext context) { - return /*1*/new Row( - children: /*2*/[ - /*3*/new RaisedButton( - onPressed: increment, - child: /*4*/new Text( - 'Increment' - )/*4:Text*/, - )/*3:RaisedButton*/, - /*5*/_makeWidget( - 'a', - 'b' - )/*5:_makeWidget*/, - new Text('Count: \$counter'), - ]/*2:List*/, - )/*1:Row*/; +void myMethod() { + return new Thing(1, + 2); } """; + var labels = await _computeElements(content); - _compareLabels(labels, content, expectedLabelCount: 5); + _compareLabels(labels, content, expectedLabelCount: 0); } - test_newConstructor() async { + /// When constructors span many like this, the node's start position is on the first line + /// of the expression and not where the opening paren is, so this test ensures we + /// dont end up with lots of unwanted labels on each line here. + test_chainedConstructorOverManyLines() async { String content = """ -void myMethod() { - return /*1*/new Class( - 1, - 2 - )/*1:Class*/; +main() { + return new thing + .whatIsSplit + .acrossManyLines(1, 2); } -"""; + """; var labels = await _computeElements(content); - _compareLabels(labels, content, expectedLabelCount: 1); + _compareLabels(labels, content, expectedLabelCount: 0); } - test_newNamedConstructor() async { + /// When chaining methods like this, the node's start position is on the first line + /// of the expression and not where the opening paren is, so this test ensures we + /// dont end up with lots of unwanted labels on each line here. + test_chainedMethodsOverManyLines() async { String content = """ -void myMethod() { - return /*1*/new Class.fromThing( - 1, - 2 - )/*1:Class.fromThing*/; +List compute() { + _unit.accept(new _DartUnitClosingLabelsComputerVisitor(this)); + return _closingLabelsByEndLine.values + .where((l) => l.any((cl) => cl.spannedLines >= 2)) + .expand((cls) => cls) + .map((clwlc) => clwlc.label) + .toList(); } -"""; + """; var labels = await _computeElements(content); - _compareLabels(labels, content, expectedLabelCount: 1); + _compareLabels(labels, content, expectedLabelCount: 0); } test_constConstructor() async { @@ -121,73 +116,100 @@ void myMethod() { _compareLabels(labels, content, expectedLabelCount: 1); } - test_staticMethod() async { + test_knownBadCode1() async { + // This code crashed during testing when I accidentally inserted a test snippet. String content = """ -void myMethod() { - return /*1*/Widget.createWidget( - 1, - 2 - )/*1:Widget.createWidget*/; +@override +Widget build(BuildContext context) { + new SliverGrid( + gridDelegate: gridDelegate, + delegate: myMethod([ + "a", + 'b', + "c", + ]), + ), + ), + ], + ), + ); } """; var labels = await _computeElements(content); - _compareLabels(labels, content, expectedLabelCount: 1); + // TODO(dantup) Results here are currently bad so this test is just checking that we + // dont crash. Need to confirm what to do here; the bad labels might not be fixed + // until the code is using the new shared parser. + // https://github.com/dart-lang/sdk/issues/30370 } - test_prefixedNewConstructor() async { + test_listLiterals() async { String content = """ -import 'dart:async' as a; void myMethod() { - return /*1*/new a.Future( + return /*1*/Widget.createWidget(/*2*/[ 1, 2 - )/*1:a.Future*/; + ]/*2:List*/)/*1:Widget.createWidget*/; } """; var labels = await _computeElements(content); - _compareLabels(labels, content, expectedLabelCount: 1); + _compareLabels(labels, content, expectedLabelCount: 2); } - test_prefixedNewNamedConstructor() async { + /// When a line contains the end of a label, we need to ensure we also include any + /// other labels that end on the same line, even if they are 1-2 lines, otherwise + /// it isn't obvious which closing bracket goes with the label. + test_mixedLineSpanning() async { String content = """ -import 'dart:async' as a; -void myMethod() { - return /*1*/new a.Future.delayed( - 1, - 2 - )/*1:a.Future.delayed*/; +main() { + /*1*/expectedLabels.forEach((m) { + /*2*/expect( + labels, + /*3*/contains( + /*4*/new ClosingLabel(expectedStart, expectedLength, expectedLabel)/*4:ClosingLabel*/)/*3:contains*/)/*2:expect*/; + })/*1:expectedLabels.forEach*/; + } } -"""; + """; var labels = await _computeElements(content); - _compareLabels(labels, content, expectedLabelCount: 1); + _compareLabels(labels, content, expectedLabelCount: 4); } - test_prefixedConstConstructor() async { + test_multipleNested() async { String content = """ -import 'dart:async' as a; -void myMethod() { - return /*1*/const a.Future( - 1, - 2 - )/*1:a.Future*/; +Widget build(BuildContext context) { + return /*1*/new Row( + children: /*2*/[ + /*3*/new RaisedButton( + onPressed: increment, + child: /*4*/new Text( + 'Increment' + )/*4:Text*/, + )/*3:RaisedButton*/, + /*5*/_makeWidget( + 'a', + 'b' + )/*5:_makeWidget*/, + new Text('Count: \$counter'), + ]/*2:List*/, + )/*1:Row*/; } """; - var labels = await _computeElements(content); - _compareLabels(labels, content, expectedLabelCount: 1); + _compareLabels(labels, content, expectedLabelCount: 5); } - test_prefixedConstNamedConstructor() async { + test_newConstructor() async { String content = """ -import 'dart:async' as a; void myMethod() { - return /*1*/const a.Future.delayed( + return /*1*/new Class( 1, 2 - )/*1:a.Future.delayed*/; + )/*1:Class*/; } """; @@ -195,14 +217,13 @@ void myMethod() { _compareLabels(labels, content, expectedLabelCount: 1); } - test_prefixedStaticMethod() async { + test_newNamedConstructor() async { String content = """ -import 'widgets.dart' as a; void myMethod() { - return /*1*/a.Widget.createWidget( + return /*1*/new Class.fromThing( 1, 2 - )/*1:a.Widget.createWidget*/; + )/*1:Class.fromThing*/; } """; @@ -210,146 +231,117 @@ void myMethod() { _compareLabels(labels, content, expectedLabelCount: 1); } - test_sameLineExcluded() async { + test_NnLabelsFromInterpolatedStrings() async { String content = """ -void myMethod() { - return new Thing(); +void main(HighlightRegionType type, int offset, int length) { + /*1*/fail( + 'Not expected to find (offset=\$offset; length=\$length; type=\$type) in\\n' + '\${regions.join('\\n')}')/*1:fail*/; } -"""; + """; var labels = await _computeElements(content); - _compareLabels(labels, content, expectedLabelCount: 0); + _compareLabels(labels, content, expectedLabelCount: 1); } - test_adjacentLinesExcluded() async { + test_prefixedConstConstructor() async { String content = """ +import 'dart:async' as a; void myMethod() { - return new Thing(1, - 2); + return /*1*/const a.Future( + 1, + 2 + )/*1:a.Future*/; } """; var labels = await _computeElements(content); - _compareLabels(labels, content, expectedLabelCount: 0); + _compareLabels(labels, content, expectedLabelCount: 1); } - test_listLiterals() async { + test_prefixedConstNamedConstructor() async { String content = """ +import 'dart:async' as a; void myMethod() { - return /*1*/Widget.createWidget(/*2*/[ + return /*1*/const a.Future.delayed( 1, 2 - ]/*2:List*/)/*1:Widget.createWidget*/; + )/*1:a.Future.delayed*/; } """; var labels = await _computeElements(content); - _compareLabels(labels, content, expectedLabelCount: 2); + _compareLabels(labels, content, expectedLabelCount: 1); } - /// When a line contains the end of a label, we need to ensure we also include any - /// other labels that end on the same line, even if they are 1-2 lines, otherwise - /// it isn't obvious which closing bracket goes with the label. - test_mixedLineSpanning() async { + test_prefixedNewConstructor() async { String content = """ -main() { - /*1*/expectedLabels.forEach((m) { - /*2*/expect( - labels, - /*3*/contains( - /*4*/new ClosingLabel(expectedStart, expectedLength, expectedLabel)/*4:ClosingLabel*/)/*3:contains*/)/*2:expect*/; - })/*1:expectedLabels.forEach*/; - } +import 'dart:async' as a; +void myMethod() { + return /*1*/new a.Future( + 1, + 2 + )/*1:a.Future*/; } - """; +"""; var labels = await _computeElements(content); - _compareLabels(labels, content, expectedLabelCount: 4); + _compareLabels(labels, content, expectedLabelCount: 1); } - /// When chaining methods like this, the node's start position is on the first line - /// of the expression and not where the opening paren is, so this test ensures we - /// dont end up with lots of unwanted labels on each line here. - test_chainedMethodsOverManyLines() async { + test_prefixedNewNamedConstructor() async { String content = """ -List compute() { - _unit.accept(new _DartUnitClosingLabelsComputerVisitor(this)); - return _closingLabelsByEndLine.values - .where((l) => l.any((cl) => cl.spannedLines >= 2)) - .expand((cls) => cls) - .map((clwlc) => clwlc.label) - .toList(); +import 'dart:async' as a; +void myMethod() { + return /*1*/new a.Future.delayed( + 1, + 2 + )/*1:a.Future.delayed*/; } - """; +"""; var labels = await _computeElements(content); - _compareLabels(labels, content, expectedLabelCount: 0); + _compareLabels(labels, content, expectedLabelCount: 1); } - /// When constructors span many like this, the node's start position is on the first line - /// of the expression and not where the opening paren is, so this test ensures we - /// dont end up with lots of unwanted labels on each line here. - test_chainedConstructorOverManyLines() async { + test_prefixedStaticMethod() async { String content = """ -main() { - return new thing - .whatIsSplit - .acrossManyLines(1, 2); +import 'widgets.dart' as a; +void myMethod() { + return /*1*/a.Widget.createWidget( + 1, + 2 + )/*1:a.Widget.createWidget*/; } - """; +"""; var labels = await _computeElements(content); - _compareLabels(labels, content, expectedLabelCount: 0); + _compareLabels(labels, content, expectedLabelCount: 1); } - test_NnLabelsFromInterpolatedStrings() async { + test_sameLineExcluded() async { String content = """ -void main(HighlightRegionType type, int offset, int length) { - /*1*/fail( - 'Not expected to find (offset=\$offset; length=\$length; type=\$type) in\\n' - '\${regions.join('\\n')}')/*1:fail*/; +void myMethod() { + return new Thing(); } - """; +"""; var labels = await _computeElements(content); - _compareLabels(labels, content, expectedLabelCount: 1); + _compareLabels(labels, content, expectedLabelCount: 0); } - test_knownBadCode1() async { - // This code crashed during testing when I accidentally inserted a test snippet. + test_staticMethod() async { String content = """ -@override -Widget build(BuildContext context) { - new SliverGrid( - gridDelegate: gridDelegate, - delegate: myMethod([ - "a", - 'b', - "c", - ]), - ), - ), - ], - ), - ); +void myMethod() { + return /*1*/Widget.createWidget( + 1, + 2 + )/*1:Widget.createWidget*/; } """; var labels = await _computeElements(content); - // TODO(dantup) Results here are currently bad so this test is just checking that we - // dont crash. Need to confirm what to do here; the bad labels might not be fixed - // until the code is using the new shared parser. - // https://github.com/dart-lang/sdk/issues/30370 - } - - Future> _computeElements(String sourceContent) async { - provider.newFile(sourcePath, sourceContent); - ResolveResult result = await driver.getResult(sourcePath); - DartUnitClosingLabelsComputer computer = - new DartUnitClosingLabelsComputer(result.lineInfo, result.unit); - return computer.compute(); + _compareLabels(labels, content, expectedLabelCount: 1); } /// Compares provided closing labels with expected @@ -386,4 +378,12 @@ Widget build(BuildContext context) { new ClosingLabel(expectedStart, expectedLength, expectedLabel))); }); } + + Future> _computeElements(String sourceContent) async { + provider.newFile(sourcePath, sourceContent); + ResolveResult result = await driver.getResult(sourcePath); + DartUnitClosingLabelsComputer computer = + new DartUnitClosingLabelsComputer(result.lineInfo, result.unit); + return computer.compute(); + } } From b6a60c44ea0687498df8e47996d9142a5446d3a7 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Fri, 11 Aug 2017 18:11:41 +0100 Subject: [PATCH 11/11] Remove unnecessary type annotations. See #30285 --- .../lib/src/computer/computer_closingLabels.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart b/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart index 830cdda20db1..ff7434768d67 100644 --- a/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart +++ b/pkg/analysis_server/lib/src/computer/computer_closingLabels.dart @@ -34,7 +34,7 @@ class _ClosingLabelWithLineCount { final ClosingLabel label; final int spannedLines; - _ClosingLabelWithLineCount(ClosingLabel this.label, int this.spannedLines); + _ClosingLabelWithLineCount(this.label, this.spannedLines); } /**