Move folding regions to actual blocks
This puts the region inside the braces; eg. the bit the user would expect to disappear when the region is collapsed.
Bug: https://github.com/dart-lang/sdk/issues/33033
Change-Id: I58b347d2e1582bb4a6a35cf45c17085ea1739816
Reviewed-on: https://dart-review.googlesource.com/54402
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Danny Tuppeny <dantup@google.com>
diff --git a/pkg/analysis_server/lib/src/analysis_server.dart b/pkg/analysis_server/lib/src/analysis_server.dart
index b838056..110ad96 100644
--- a/pkg/analysis_server/lib/src/analysis_server.dart
+++ b/pkg/analysis_server/lib/src/analysis_server.dart
@@ -1275,15 +1275,13 @@
analysisServer, path, result.lineInfo, unit);
});
}
- // TODO:(dantup) Uncomment this and equivilent in
- // test/analysis/notification_folding_test.dart once the
- // implementation is complete.
- // if (analysisServer._hasAnalysisServiceSubscription(
- // AnalysisService.FOLDING, path)) {
- // _runDelayed(() {
- // sendAnalysisNotificationFolding(analysisServer, path, unit);
- // });
- // }
+ if (analysisServer._hasAnalysisServiceSubscription(
+ AnalysisService.FOLDING, path)) {
+ _runDelayed(() {
+ sendAnalysisNotificationFolding(
+ analysisServer, path, result.lineInfo, unit);
+ });
+ }
if (analysisServer._hasAnalysisServiceSubscription(
AnalysisService.OUTLINE, path)) {
_runDelayed(() {
diff --git a/pkg/analysis_server/lib/src/computer/computer_folding.dart b/pkg/analysis_server/lib/src/computer/computer_folding.dart
index 19b0c0d..cf76107 100644
--- a/pkg/analysis_server/lib/src/computer/computer_folding.dart
+++ b/pkg/analysis_server/lib/src/computer/computer_folding.dart
@@ -3,19 +3,22 @@
// BSD-style license that can be found in the LICENSE file.
import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/ast/syntactic_entity.dart';
import 'package:analyzer/dart/ast/visitor.dart';
+import 'package:analyzer/src/generated/source.dart';
import 'package:analyzer_plugin/protocol/protocol_common.dart';
/**
* A computer for [CompilationUnit] folding.
*/
class DartUnitFoldingComputer {
+ final LineInfo _lineInfo;
final CompilationUnit _unit;
Directive _firstDirective, _lastDirective;
final List<FoldingRegion> _foldingRegions = [];
- DartUnitFoldingComputer(this._unit);
+ DartUnitFoldingComputer(this._lineInfo, this._unit);
/**
* Returns a list of folding regions, not `null`.
@@ -26,8 +29,10 @@
if (_firstDirective != null &&
_lastDirective != null &&
_firstDirective != _lastDirective) {
- _foldingRegions.add(new FoldingRegion(FoldingKind.DIRECTIVES,
- _firstDirective.offset, _lastDirective.end - _firstDirective.offset));
+ _foldingRegions.add(new FoldingRegion(
+ FoldingKind.DIRECTIVES,
+ _firstDirective.keyword.end,
+ _lastDirective.end - _firstDirective.keyword.end));
}
return _foldingRegions;
@@ -42,11 +47,76 @@
_DartUnitFoldingComputerVisitor(this._computer);
@override
+ Object visitBlockFunctionBody(BlockFunctionBody node) {
+ final FoldingKind kind = node.parent is ConstructorDeclaration ||
+ node.parent is MethodDeclaration
+ ? FoldingKind.CLASS_MEMBER
+ : FoldingKind.TOP_LEVEL_DECLARATION;
+ _addRegion(
+ node.block.leftBracket.end, node.block.rightBracket.offset, kind);
+ return super.visitBlockFunctionBody(node);
+ }
+
+ @override
+ Object visitClassDeclaration(ClassDeclaration node) {
+ _addRegion(node.leftBracket.end, node.rightBracket.offset,
+ FoldingKind.TOP_LEVEL_DECLARATION);
+ return super.visitClassDeclaration(node);
+ }
+
+ @override
+ Object visitComment(Comment node) {
+ final FoldingKind kind = node.isDocumentation
+ ? FoldingKind.DOCUMENTATION_COMMENT
+ : FoldingKind.COMMENT;
+ _addRegion(node.offset, node.end, kind);
+ return super.visitComment(node);
+ }
+
+ @override
+ Object visitExportDirective(ExportDirective node) {
+ _recordDirective(node);
+ return super.visitExportDirective(node);
+ }
+
+ @override
visitImportDirective(ImportDirective node) {
- if (_computer._firstDirective == null) {
- _computer._firstDirective = node;
- }
- _computer._lastDirective = node;
+ _recordDirective(node);
return super.visitImportDirective(node);
}
+
+ @override
+ Object visitLibraryDirective(LibraryDirective node) {
+ _recordDirective(node);
+ return super.visitLibraryDirective(node);
+ }
+
+ @override
+ Object visitPartDirective(PartDirective node) {
+ _recordDirective(node);
+ return super.visitPartDirective(node);
+ }
+
+ @override
+ Object visitPartOfDirective(PartOfDirective node) {
+ _recordDirective(node);
+ return super.visitPartOfDirective(node);
+ }
+
+ _addRegion(int startOffset, int endOffset, FoldingKind kind) {
+ // TODO(dantup): This class is marked deprecated; find out what to change it to.
+ final LineInfo_Location start =
+ _computer._lineInfo.getLocation(startOffset);
+ final LineInfo_Location end = _computer._lineInfo.getLocation(endOffset);
+
+ if (start.lineNumber != end.lineNumber) {
+ _computer._foldingRegions
+ .add(new FoldingRegion(kind, startOffset, endOffset - startOffset));
+ }
+ }
+
+ _recordDirective(Directive node) {
+ _computer._firstDirective ??= node;
+ _computer._lastDirective = node;
+ }
}
diff --git a/pkg/analysis_server/lib/src/operation/operation_analysis.dart b/pkg/analysis_server/lib/src/operation/operation_analysis.dart
index c08b148..24a4811 100644
--- a/pkg/analysis_server/lib/src/operation/operation_analysis.dart
+++ b/pkg/analysis_server/lib/src/operation/operation_analysis.dart
@@ -81,10 +81,10 @@
});
}
-void sendAnalysisNotificationFolding(
- AnalysisServer server, String file, CompilationUnit dartUnit) {
+void sendAnalysisNotificationFolding(AnalysisServer server, String file,
+ LineInfo lineInfo, CompilationUnit dartUnit) {
_sendNotification(server, () {
- var regions = new DartUnitFoldingComputer(dartUnit).compute();
+ var regions = new DartUnitFoldingComputer(lineInfo, dartUnit).compute();
var params = new protocol.AnalysisFoldingParams(file, regions);
server.sendNotification(params.toNotification());
});
diff --git a/pkg/analysis_server/test/analysis/notification_folding_test.dart b/pkg/analysis_server/test/analysis/notification_folding_test.dart
index 51a400b..6d43ddb 100644
--- a/pkg/analysis_server/test/analysis/notification_folding_test.dart
+++ b/pkg/analysis_server/test/analysis/notification_folding_test.dart
@@ -15,11 +15,7 @@
main() {
defineReflectiveSuite(() {
- // TODO(dantup): Uncomment once implementation is complete.
- // Cannot just mark the tests as @failingTest as they time out
- // (no FOLDING notification ever) and failingTest doesn't seem
- // to cover that.
- // defineReflectiveTests(_AnalysisNotificationFoldingTest);
+ defineReflectiveTests(_AnalysisNotificationFoldingTest);
});
}
@@ -33,7 +29,9 @@
''';
static final expectedResults = [
- new FoldingRegion(FoldingKind.DIRECTIVES, 0, 40)
+ // We don't include the first "import" in the region because
+ // we want that to remain visible (not collapse).
+ new FoldingRegion(FoldingKind.DIRECTIVES, 6, 34)
];
List<FoldingRegion> lastRegions;
diff --git a/pkg/analysis_server/test/src/computer/folding_computer_test.dart b/pkg/analysis_server/test/src/computer/folding_computer_test.dart
index c9d00a1..171926b 100644
--- a/pkg/analysis_server/test/src/computer/folding_computer_test.dart
+++ b/pkg/analysis_server/test/src/computer/folding_computer_test.dart
@@ -41,13 +41,13 @@
test_multiple_import_directives() async {
String content = """
-/*1*/import 'dart:async';
+import/*1:INC*/ 'dart:async';
// We can have comments
import 'package:a/b.dart';
import 'package:b/c.dart';
-import '../a.dart';/*1:DIRECTIVES*/
+import '../a.dart';/*1:EXC:DIRECTIVES*/
main() {}
""";
@@ -56,11 +56,99 @@
_compareRegions(regions, content);
}
+ test_multiple_directive_types() async {
+ String content = """
+import/*1:INC*/ 'dart:async';
+
+// We can have comments
+import 'package:a/b.dart';
+import 'package:b/c.dart';
+
+export '../a.dart';/*1:EXC:DIRECTIVES*/
+
+main() {}
+""";
+
+ final regions = await _computeRegions(content);
+ _compareRegions(regions, content);
+ }
+
+ test_function() async {
+ String content = """
+// Content before
+
+main() {/*1:INC*/
+ print("Hello, world!");
+/*1:INC:TOP_LEVEL_DECLARATION*/}
+
+// Content after
+""";
+
+ final regions = await _computeRegions(content);
+ _compareRegions(regions, content);
+ }
+
+ test_function_with_dart_doc() async {
+ String content = """
+// Content before
+
+/*1:EXC*//// This is a doc comment
+/// that spans lines/*1:INC:DOCUMENTATION_COMMENT*/
+main() {/*2:INC*/
+ print("Hello, world!");
+/*2:INC:TOP_LEVEL_DECLARATION*/}
+
+// Content after
+""";
+
+ final regions = await _computeRegions(content);
+ _compareRegions(regions, content);
+ }
+
+ test_nested_function() async {
+ String content = """
+// Content before
+
+main() {/*1:INC*/
+ doPrint() {/*2:INC*/
+ print("Hello, world!");
+ /*2:INC:TOP_LEVEL_DECLARATION*/}
+ doPrint();
+/*1:INC:TOP_LEVEL_DECLARATION*/}
+
+// Content after
+""";
+
+ final regions = await _computeRegions(content);
+ _compareRegions(regions, content);
+ }
+
+ test_class() async {
+ String content = """
+// Content before
+
+class Person {/*1:INC*/
+ Person() {/*2:INC*/
+ print("Hello, world!");
+ /*2:INC:CLASS_MEMBER*/}
+
+ void sayHello() {/*3:INC*/
+ print("Hello, world!");
+ /*3:INC:CLASS_MEMBER*/}
+/*1:INC:TOP_LEVEL_DECLARATION*/}
+
+// Content after
+""";
+
+ final regions = await _computeRegions(content);
+ _compareRegions(regions, content);
+ }
+
/// Compares provided folding regions with expected
/// regions extracted from the comments in the provided content.
void _compareRegions(List<FoldingRegion> regions, String content) {
// Find all numeric markers for region starts.
- final regex = new RegExp(r'/\*(\d+)\*/');
+ final regex = new RegExp(r'/\*(\d+):(INC|EXC)\*/');
final expectedRegions = regex.allMatches(content);
// Check we didn't get more than expected, since the loop below only
@@ -71,14 +159,22 @@
// ensure it's in the results.
expectedRegions.forEach((m) {
final i = m.group(1);
+ final inclusiveStart = m.group(2) == "INC";
// Find the end marker.
- final endMatch = new RegExp('/\\*$i:(.+?)\\*/').firstMatch(content);
+ final endMatch =
+ new RegExp('/\\*$i:(INC|EXC):(.+?)\\*/').firstMatch(content);
- final expectedStart = m.end;
- final expectedLength = endMatch.start - expectedStart;
- final expectedKindString = endMatch.group(1);
- final expectedKind = FoldingKind.VALUES
- .firstWhere((f) => f.toString() == 'FoldingKind.$expectedKindString');
+ final inclusiveEnd = endMatch.group(1) == "INC";
+ final expectedKindString = endMatch.group(2);
+ final expectedKind = FoldingKind.VALUES.firstWhere(
+ (f) => f.toString() == 'FoldingKind.$expectedKindString',
+ orElse: () => throw new Exception(
+ "Annotated test code references $expectedKindString but "
+ "this does not exist in FoldingKind"));
+
+ final expectedStart = inclusiveStart ? m.start : m.end;
+ final expectedLength =
+ (inclusiveEnd ? endMatch.end : endMatch.start) - expectedStart;
expect(
regions,
@@ -90,7 +186,8 @@
Future<List<FoldingRegion>> _computeRegions(String sourceContent) async {
newFile(sourcePath, content: sourceContent);
ResolveResult result = await driver.getResult(sourcePath);
- DartUnitFoldingComputer computer = new DartUnitFoldingComputer(result.unit);
+ DartUnitFoldingComputer computer =
+ new DartUnitFoldingComputer(result.lineInfo, result.unit);
return computer.compute();
}
}
