Skip to content

Commit

Permalink
Move folding regions to actual blocks
Browse files Browse the repository at this point in the history 
This puts the region inside the braces; eg. the bit the user would expect to disappear when the region is collapsed.

Bug: #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>
  • Loading branch information
@DanTup
DanTup authored and commit-bot@chromium.org committed May 10, 2018
1 parent 407872b commit 4604300
Show file tree
Hide file tree
Showing 5 changed files with 197 additions and 34 deletions.
16 changes: 7 additions & 9 deletions pkg/analysis_server/lib/src/analysis_server.dart
View file
Expand Up @@ -1275,15 +1275,13 @@ class ServerContextManagerCallbacks extends ContextManagerCallbacks {
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(() {
Expand Down
82 changes: 76 additions & 6 deletions pkg/analysis_server/lib/src/computer/computer_folding.dart
View file
Expand Up @@ -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`.
Expand All @@ -26,8 +29,10 @@ class DartUnitFoldingComputer {
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;
Expand All @@ -41,12 +46,77 @@ class _DartUnitFoldingComputerVisitor extends RecursiveAstVisitor<Object> {
final DartUnitFoldingComputer _computer;
_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;
_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;
return super.visitImportDirective(node);
}
}
6 changes: 3 additions & 3 deletions pkg/analysis_server/lib/src/operation/operation_analysis.dart
View file
Expand Up @@ -81,10 +81,10 @@ void sendAnalysisNotificationClosingLabels(AnalysisServer server, String file,
});
}

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());
});
Expand Down
10 changes: 4 additions & 6 deletions pkg/analysis_server/test/analysis/notification_folding_test.dart
View file
Expand Up @@ -15,11 +15,7 @@ import '../analysis_abstract.dart';

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);
});
}

Expand All @@ -33,7 +29,9 @@ main async() {}
''';

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;
Expand Down
117 changes: 107 additions & 10 deletions pkg/analysis_server/test/src/computer/folding_computer_test.dart
View file
Expand Up @@ -41,13 +41,13 @@ main() {}

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() {}
""";
Expand All @@ -56,11 +56,99 @@ main() {}
_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
Expand All @@ -71,14 +159,22 @@ main() {}
// 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 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 = m.end;
final expectedLength = endMatch.start - expectedStart;
final expectedKindString = endMatch.group(1);
final expectedKind = FoldingKind.VALUES
.firstWhere((f) => f.toString() == 'FoldingKind.$expectedKindString');
final expectedStart = inclusiveStart ? m.start : m.end;
final expectedLength =
(inclusiveEnd ? endMatch.end : endMatch.start) - expectedStart;

expect(
regions,
Expand All @@ -90,7 +186,8 @@ main() {}
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();
}
}

0 comments on commit 4604300

Please sign in to comment.