From 5cffc0d44ff30418b15fd844c52ac1f4c448c0b8 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Wed, 20 Feb 2019 12:09:20 -0800 Subject: [PATCH 1/4] Fix line number cache and add tests --- lib/src/line_number_cache.dart | 18 +++++----- test/line_number_cache_test.dart | 57 ++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 10 deletions(-) create mode 100644 test/line_number_cache_test.dart diff --git a/lib/src/line_number_cache.dart b/lib/src/line_number_cache.dart index 300023808e..1708e53cd3 100644 --- a/lib/src/line_number_cache.dart +++ b/lib/src/line_number_cache.dart @@ -8,6 +8,7 @@ import 'dart:collection'; import 'dart:io'; import 'package:dartdoc/src/tuple.dart'; +import 'package:path/path.dart' as pathLib; String _getNewlineChar(String contents) { if (contents.contains("\r\n")) { @@ -22,12 +23,12 @@ String _getNewlineChar(String contents) { SplayTreeMap _createLineNumbersMap(String contents) { var newlineChar = _getNewlineChar(contents); var offset = 0; - var lineNumber = 0; + var lineNumber = 1; var result = new SplayTreeMap(); do { result[offset] = lineNumber; - offset = contents.indexOf(newlineChar, offset + 1); + offset = (offset + 1 <= contents.length) ? contents.indexOf(newlineChar, offset + 1) : -1; lineNumber += 1; } while (offset != -1); @@ -36,22 +37,19 @@ SplayTreeMap _createLineNumbersMap(String contents) { final LineNumberCache lineNumberCache = new LineNumberCache(); -// TODO(kevmoo): this could use some testing class LineNumberCache { - final Map __fileContents = {}; final Map> _lineNumbers = >{}; Tuple2 lineAndColumn(String file, int offset) { + file = pathLib.canonicalize(file); var lineMap = _lineNumbers.putIfAbsent( - file, () => _createLineNumbersMap(_fileContents(file))); + file, () => _createLineNumbersMap(new File(file).readAsStringSync())); var lastKey = lineMap.lastKeyBefore(offset); if (lastKey != null) { - return new Tuple2(lineMap[lastKey] + 1, offset - lastKey); + return new Tuple2(lineMap[lastKey], offset - lastKey); + } else { + return new Tuple2(lineMap[0], offset - 0); } - return null; } - - String _fileContents(String file) => - __fileContents.putIfAbsent(file, () => new File(file).readAsStringSync()); } diff --git a/test/line_number_cache_test.dart b/test/line_number_cache_test.dart new file mode 100644 index 0000000000..a7600259c8 --- /dev/null +++ b/test/line_number_cache_test.dart @@ -0,0 +1,57 @@ +// Copyright (c) 2016, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +library dartdoc.cache_test; + +import 'dart:io'; + +import 'package:dartdoc/src/line_number_cache.dart'; +import 'package:path/path.dart' as pathLib; +import 'package:test/test.dart'; +import 'package:dartdoc/src/tuple.dart'; + +void main() { + group('Verify basic line cache behavior', () { + Directory _tempDir; + + setUp(() { + _tempDir = Directory.systemTemp.createTempSync('line_number_cache'); + }); + + tearDown(() { + _tempDir.deleteSync(recursive: true); + }); + + test('validate empty file behavior', () { + File emptyFile = File(pathLib.join(_tempDir.path, 'empty_file'))..writeAsStringSync(''); + LineNumberCache cache = LineNumberCache(); + expect(cache.lineAndColumn(emptyFile.path, 0), equals(Tuple2(1, 0))); + }); + + test('single line without newline', () { + File singleLineWithoutNewline = File(pathLib.join(_tempDir.path, 'single_line'))..writeAsStringSync('a single line'); + LineNumberCache cache = LineNumberCache(); + expect(cache.lineAndColumn(singleLineWithoutNewline.path, 2), equals(Tuple2(1, 2))); + expect(cache.lineAndColumn(singleLineWithoutNewline.path, 0), equals(Tuple2(1, 0))); + }); + + test('multiple line without trailing newline', () { + File multipleLine = File(pathLib.join(_tempDir.path, 'multiple_line'))..writeAsStringSync('This is the first line\nThis is the second line'); + LineNumberCache cache = LineNumberCache(); + expect(cache.lineAndColumn(multipleLine.path, 60), equals(Tuple2(2, 38))); + expect(cache.lineAndColumn(multipleLine.path, 30), equals(Tuple2(2, 8))); + expect(cache.lineAndColumn(multipleLine.path, 5), equals(Tuple2(1, 5))); + expect(cache.lineAndColumn(multipleLine.path, 0), equals(Tuple2(1, 0))); + }); + + test('multiple lines with trailing newline', () { + File multipleLine = File(pathLib.join(_tempDir.path, 'multiple_line'))..writeAsStringSync('This is the first line\nThis is the second line\n'); + LineNumberCache cache = LineNumberCache(); + expect(cache.lineAndColumn(multipleLine.path, 60), equals(Tuple2(3, 14))); + expect(cache.lineAndColumn(multipleLine.path, 30), equals(Tuple2(2, 8))); + expect(cache.lineAndColumn(multipleLine.path, 5), equals(Tuple2(1, 5))); + expect(cache.lineAndColumn(multipleLine.path, 0), equals(Tuple2(1, 0))); + }); + }); +} \ No newline at end of file From 4d85b815b022eea0fd5a54598c8fa85c22c44a7d Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Wed, 20 Feb 2019 12:09:32 -0800 Subject: [PATCH 2/4] dartfmt --- lib/src/line_number_cache.dart | 4 +- lib/src/model.dart | 7 ++- test/line_number_cache_test.dart | 22 ++++--- test/model_test.dart | 57 ++++++++++++------- test/src/utils.dart | 7 ++- testing/test_package/lib/fake.dart | 3 +- .../test_package_experiments/lib/main.dart | 5 +- 7 files changed, 69 insertions(+), 36 deletions(-) diff --git a/lib/src/line_number_cache.dart b/lib/src/line_number_cache.dart index 1708e53cd3..2381b93bce 100644 --- a/lib/src/line_number_cache.dart +++ b/lib/src/line_number_cache.dart @@ -28,7 +28,9 @@ SplayTreeMap _createLineNumbersMap(String contents) { do { result[offset] = lineNumber; - offset = (offset + 1 <= contents.length) ? contents.indexOf(newlineChar, offset + 1) : -1; + offset = (offset + 1 <= contents.length) + ? contents.indexOf(newlineChar, offset + 1) + : -1; lineNumber += 1; } while (offset != -1); diff --git a/lib/src/model.dart b/lib/src/model.dart index 8e03022b13..79db1c2648 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -4701,9 +4701,10 @@ class PackageGraph { Set precachedElements = new Set(); Iterable precacheOneElement(ModelElement m) sync* { - for (ModelElement d in m.documentationFrom.where((d) => d.documentationComment != null)) { - if (needsPrecacheRegExp.hasMatch(d.documentationComment) - && !precachedElements.contains(d)) { + for (ModelElement d + in m.documentationFrom.where((d) => d.documentationComment != null)) { + if (needsPrecacheRegExp.hasMatch(d.documentationComment) && + !precachedElements.contains(d)) { precachedElements.add(d); yield d._precacheLocalDocs(); } diff --git a/test/line_number_cache_test.dart b/test/line_number_cache_test.dart index a7600259c8..f9c767520d 100644 --- a/test/line_number_cache_test.dart +++ b/test/line_number_cache_test.dart @@ -24,20 +24,26 @@ void main() { }); test('validate empty file behavior', () { - File emptyFile = File(pathLib.join(_tempDir.path, 'empty_file'))..writeAsStringSync(''); + File emptyFile = File(pathLib.join(_tempDir.path, 'empty_file')) + ..writeAsStringSync(''); LineNumberCache cache = LineNumberCache(); expect(cache.lineAndColumn(emptyFile.path, 0), equals(Tuple2(1, 0))); }); test('single line without newline', () { - File singleLineWithoutNewline = File(pathLib.join(_tempDir.path, 'single_line'))..writeAsStringSync('a single line'); + File singleLineWithoutNewline = + File(pathLib.join(_tempDir.path, 'single_line')) + ..writeAsStringSync('a single line'); LineNumberCache cache = LineNumberCache(); - expect(cache.lineAndColumn(singleLineWithoutNewline.path, 2), equals(Tuple2(1, 2))); - expect(cache.lineAndColumn(singleLineWithoutNewline.path, 0), equals(Tuple2(1, 0))); + expect(cache.lineAndColumn(singleLineWithoutNewline.path, 2), + equals(Tuple2(1, 2))); + expect(cache.lineAndColumn(singleLineWithoutNewline.path, 0), + equals(Tuple2(1, 0))); }); test('multiple line without trailing newline', () { - File multipleLine = File(pathLib.join(_tempDir.path, 'multiple_line'))..writeAsStringSync('This is the first line\nThis is the second line'); + File multipleLine = File(pathLib.join(_tempDir.path, 'multiple_line')) + ..writeAsStringSync('This is the first line\nThis is the second line'); LineNumberCache cache = LineNumberCache(); expect(cache.lineAndColumn(multipleLine.path, 60), equals(Tuple2(2, 38))); expect(cache.lineAndColumn(multipleLine.path, 30), equals(Tuple2(2, 8))); @@ -46,7 +52,9 @@ void main() { }); test('multiple lines with trailing newline', () { - File multipleLine = File(pathLib.join(_tempDir.path, 'multiple_line'))..writeAsStringSync('This is the first line\nThis is the second line\n'); + File multipleLine = File(pathLib.join(_tempDir.path, 'multiple_line')) + ..writeAsStringSync( + 'This is the first line\nThis is the second line\n'); LineNumberCache cache = LineNumberCache(); expect(cache.lineAndColumn(multipleLine.path, 60), equals(Tuple2(3, 14))); expect(cache.lineAndColumn(multipleLine.path, 30), equals(Tuple2(2, 8))); @@ -54,4 +62,4 @@ void main() { expect(cache.lineAndColumn(multipleLine.path, 0), equals(Tuple2(1, 0))); }); }); -} \ No newline at end of file +} diff --git a/test/model_test.dart b/test/model_test.dart index f35c08a024..57c5747418 100644 --- a/test/model_test.dart +++ b/test/model_test.dart @@ -88,12 +88,18 @@ void main() { // when the feature is enabled by default. group('Experiments', () { Library main; - TopLevelVariable aComplexSet, inferredTypeSet, specifiedSet, untypedMap, typedSet; + TopLevelVariable aComplexSet, + inferredTypeSet, + specifiedSet, + untypedMap, + typedSet; setUpAll(() { - main = packageGraphExperiments.libraries.firstWhere((lib) => lib.name == 'main'); + main = packageGraphExperiments.libraries + .firstWhere((lib) => lib.name == 'main'); aComplexSet = main.constants.firstWhere((v) => v.name == 'aComplexSet'); - inferredTypeSet = main.constants.firstWhere((v) => v.name == 'inferredTypeSet'); + inferredTypeSet = + main.constants.firstWhere((v) => v.name == 'inferredTypeSet'); specifiedSet = main.constants.firstWhere((v) => v.name == 'specifiedSet'); untypedMap = main.constants.firstWhere((v) => v.name == 'untypedMap'); typedSet = main.constants.firstWhere((v) => v.name == 'typedSet'); @@ -101,19 +107,26 @@ void main() { test('Set literals test', () { expect(aComplexSet.modelType.name, equals('Set')); - expect(aComplexSet.modelType.typeArguments.map((a) => a.name).toList(), equals(['AClassContainingLiterals'])); - expect(aComplexSet.constantValue, equals('const {const AClassContainingLiterals(3, 5)}')); + expect(aComplexSet.modelType.typeArguments.map((a) => a.name).toList(), + equals(['AClassContainingLiterals'])); + expect(aComplexSet.constantValue, + equals('const {const AClassContainingLiterals(3, 5)}')); expect(inferredTypeSet.modelType.name, equals('Set')); - expect(inferredTypeSet.modelType.typeArguments.map((a) => a.name).toList(), equals(['num'])); + expect( + inferredTypeSet.modelType.typeArguments.map((a) => a.name).toList(), + equals(['num'])); expect(inferredTypeSet.constantValue, equals('const {1, 2.5, 3}')); expect(specifiedSet.modelType.name, equals('Set')); - expect(specifiedSet.modelType.typeArguments.map((a) => a.name).toList(), equals(['int'])); + expect(specifiedSet.modelType.typeArguments.map((a) => a.name).toList(), + equals(['int'])); expect(specifiedSet.constantValue, equals('const {}')); expect(untypedMap.modelType.name, equals('Map')); - expect(untypedMap.modelType.typeArguments.map((a) => a.name).toList(), equals(['dynamic', 'dynamic'])); + expect(untypedMap.modelType.typeArguments.map((a) => a.name).toList(), + equals(['dynamic', 'dynamic'])); expect(untypedMap.constantValue, equals('const {}')); expect(typedSet.modelType.name, equals('Set')); - expect(typedSet.modelType.typeArguments.map((a) => a.name).toList(), equals(['String'])); + expect(typedSet.modelType.typeArguments.map((a) => a.name).toList(), + equals(['String'])); expect(typedSet.constantValue, equals('const <String> {}')); }); }); @@ -128,7 +141,8 @@ void main() { Method invokeToolNonCanonical, invokeToolNonCanonicalSubclass; Method invokeToolPrivateLibrary, invokeToolPrivateLibraryOriginal; Method invokeToolParentDoc, invokeToolParentDocOriginal; - final RegExp packageInvocationIndexRegexp = new RegExp(r'PACKAGE_INVOCATION_INDEX: (\d+)'); + final RegExp packageInvocationIndexRegexp = + new RegExp(r'PACKAGE_INVOCATION_INDEX: (\d+)'); setUpAll(() { _NonCanonicalToolUser = fakeLibrary.allClasses @@ -165,13 +179,16 @@ void main() { packageGraph.allLocalModelElements.forEach((m) => m.documentation); }); - test('invokes tool when inherited documentation is the only means for it to be seen', () { + test( + 'invokes tool when inherited documentation is the only means for it to be seen', + () { // Verify setup of the test is correct. expect(invokeToolParentDoc.isCanonical, isTrue); expect(invokeToolParentDoc.documentationComment, isNull); // Error message here might look strange due to toString() on Methods, but if this // fails that means we don't have the correct invokeToolParentDoc instance. - expect(invokeToolParentDoc.documentationFrom, contains(invokeToolParentDocOriginal)); + expect(invokeToolParentDoc.documentationFrom, + contains(invokeToolParentDocOriginal)); // Tool should be substituted out here. expect(invokeToolParentDoc.documentation, isNot(contains('{@tool'))); }); @@ -187,18 +204,20 @@ void main() { equals(packageInvocationIndexRegexp .firstMatch(invokeToolNonCanonicalSubclass.documentation) .group(1))); - expect(invokeToolPrivateLibrary.documentation, isNot(contains('{@tool'))); + expect( + invokeToolPrivateLibrary.documentation, isNot(contains('{@tool'))); expect( invokeToolPrivateLibraryOriginal.documentation, contains('{@tool')); }); test('Documentation borrowed from implementer case', () { - expect(packageInvocationIndexRegexp - .firstMatch(invokeToolParentDoc.documentation) - .group(1), - equals(packageInvocationIndexRegexp - .firstMatch(invokeToolParentDocOriginal.documentation) - .group(1))); + expect( + packageInvocationIndexRegexp + .firstMatch(invokeToolParentDoc.documentation) + .group(1), + equals(packageInvocationIndexRegexp + .firstMatch(invokeToolParentDocOriginal.documentation) + .group(1))); }); }); diff --git a/test/src/utils.dart b/test/src/utils.dart index ff6f728953..86a0d8fe2f 100644 --- a/test/src/utils.dart +++ b/test/src/utils.dart @@ -29,7 +29,8 @@ PackageGraph testPackageGraphSdk; final Directory testPackageBadDir = new Directory('testing/test_package_bad'); final Directory testPackageDir = new Directory('testing/test_package'); -final Directory testPackageExperimentsDir = new Directory('testing/test_package_experiments'); +final Directory testPackageExperimentsDir = + new Directory('testing/test_package_experiments'); final Directory testPackageMinimumDir = new Directory('testing/test_package_minimum'); final Directory testPackageWithEmbedderYaml = @@ -82,8 +83,8 @@ void init({List additionalArguments}) async { testPackageGraphExperiments = await bootBasicPackage( 'testing/test_package_experiments', [], - additionalArguments: additionalArguments + ['--enable-experiment', 'set-literals'] - ); + additionalArguments: + additionalArguments + ['--enable-experiment', 'set-literals']); testPackageGraphSmall = await bootBasicPackage( 'testing/test_package_small', [], diff --git a/testing/test_package/lib/fake.dart b/testing/test_package/lib/fake.dart index 248f7d9895..61091f2565 100644 --- a/testing/test_package/lib/fake.dart +++ b/testing/test_package/lib/fake.dart @@ -1032,7 +1032,8 @@ abstract class ImplementingClassForTool { } /// The method [invokeToolParentDoc] gets its documentation from an interface class. -abstract class CanonicalPrivateInheritedToolUser implements ImplementingClassForTool { +abstract class CanonicalPrivateInheritedToolUser + implements ImplementingClassForTool { @override void invokeToolParentDoc() { print('hello, tool world'); diff --git a/testing/test_package_experiments/lib/main.dart b/testing/test_package_experiments/lib/main.dart index 23c65c0036..07e02a6b9e 100644 --- a/testing/test_package_experiments/lib/main.dart +++ b/testing/test_package_experiments/lib/main.dart @@ -10,7 +10,8 @@ class AClassContainingLiterals { const AClassContainingLiterals(this.value1, this.value2); @override - bool operator==(Object other) => other is AClassContainingLiterals && value1 == other.value1; + bool operator ==(Object other) => + other is AClassContainingLiterals && value1 == other.value1; } -const aComplexSet = {AClassContainingLiterals(3, 5)}; \ No newline at end of file +const aComplexSet = {AClassContainingLiterals(3, 5)}; From 0ad75e09dd2d3c50619859956a2fdd3688c46192 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Wed, 20 Feb 2019 15:34:28 -0800 Subject: [PATCH 3/4] remove accidental minus 0 --- lib/src/line_number_cache.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/line_number_cache.dart b/lib/src/line_number_cache.dart index 2381b93bce..3479d91964 100644 --- a/lib/src/line_number_cache.dart +++ b/lib/src/line_number_cache.dart @@ -51,7 +51,7 @@ class LineNumberCache { if (lastKey != null) { return new Tuple2(lineMap[lastKey], offset - lastKey); } else { - return new Tuple2(lineMap[0], offset - 0); + return new Tuple2(lineMap[0], offset); } } } From 234b423a1d3c60767154bad3998f6f1abc84eba9 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Mon, 30 Sep 2019 13:12:03 -0700 Subject: [PATCH 4/4] Review comments and lint fixup for modern Dart --- lib/src/line_number_cache.dart | 4 ++-- test/line_number_cache_test.dart | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/src/line_number_cache.dart b/lib/src/line_number_cache.dart index 4ffc83e9e1..3190d5a316 100644 --- a/lib/src/line_number_cache.dart +++ b/lib/src/line_number_cache.dart @@ -8,7 +8,7 @@ import 'dart:collection'; import 'dart:io'; import 'package:dartdoc/src/tuple.dart'; -import 'package:path/path.dart' as pathLib; +import 'package:path/path.dart' as path; String _getNewlineChar(String contents) { if (contents.contains("\r\n")) { @@ -44,7 +44,7 @@ class LineNumberCache { >{}; Tuple2 lineAndColumn(String file, int offset) { - file = pathLib.canonicalize(file); + file = path.canonicalize(file); var lineMap = _lineNumbers.putIfAbsent( file, () => _createLineNumbersMap(File(file).readAsStringSync())); var lastKey = lineMap.lastKeyBefore(offset); diff --git a/test/line_number_cache_test.dart b/test/line_number_cache_test.dart index f9c767520d..e013ce459a 100644 --- a/test/line_number_cache_test.dart +++ b/test/line_number_cache_test.dart @@ -1,4 +1,4 @@ -// Copyright (c) 2016, the Dart project authors. Please see the AUTHORS file +// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. @@ -7,9 +7,9 @@ library dartdoc.cache_test; import 'dart:io'; import 'package:dartdoc/src/line_number_cache.dart'; -import 'package:path/path.dart' as pathLib; -import 'package:test/test.dart'; import 'package:dartdoc/src/tuple.dart'; +import 'package:path/path.dart' as path; +import 'package:test/test.dart'; void main() { group('Verify basic line cache behavior', () { @@ -24,7 +24,7 @@ void main() { }); test('validate empty file behavior', () { - File emptyFile = File(pathLib.join(_tempDir.path, 'empty_file')) + File emptyFile = File(path.join(_tempDir.path, 'empty_file')) ..writeAsStringSync(''); LineNumberCache cache = LineNumberCache(); expect(cache.lineAndColumn(emptyFile.path, 0), equals(Tuple2(1, 0))); @@ -32,7 +32,7 @@ void main() { test('single line without newline', () { File singleLineWithoutNewline = - File(pathLib.join(_tempDir.path, 'single_line')) + File(path.join(_tempDir.path, 'single_line')) ..writeAsStringSync('a single line'); LineNumberCache cache = LineNumberCache(); expect(cache.lineAndColumn(singleLineWithoutNewline.path, 2), @@ -42,7 +42,7 @@ void main() { }); test('multiple line without trailing newline', () { - File multipleLine = File(pathLib.join(_tempDir.path, 'multiple_line')) + File multipleLine = File(path.join(_tempDir.path, 'multiple_line')) ..writeAsStringSync('This is the first line\nThis is the second line'); LineNumberCache cache = LineNumberCache(); expect(cache.lineAndColumn(multipleLine.path, 60), equals(Tuple2(2, 38))); @@ -52,7 +52,7 @@ void main() { }); test('multiple lines with trailing newline', () { - File multipleLine = File(pathLib.join(_tempDir.path, 'multiple_line')) + File multipleLine = File(path.join(_tempDir.path, 'multiple_line')) ..writeAsStringSync( 'This is the first line\nThis is the second line\n'); LineNumberCache cache = LineNumberCache();