Skip to content

Commit

Permalink
file_name check improvements; benchmarking (dart-lang#1829)
Browse files Browse the repository at this point in the history
* file_name check improvements; benchmarking

* inlining and cleanup

* tidy
  • Loading branch information
pq authored Nov 12, 2019
1 parent ba0d059 commit 30d0e92
Show file tree
Hide file tree
Showing 8 changed files with 265 additions and 24 deletions.
7 changes: 3 additions & 4 deletions lib/src/rules/file_names.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/visitor.dart';

import '../analyzer.dart';
import '../utils.dart';
import '../util/ascii_utils.dart';

const _desc = r'Name source files using `lowercase_with_underscores`.';

Expand Down Expand Up @@ -66,9 +66,8 @@ class _Visitor extends SimpleAstVisitor<void> {

@override
void visitCompilationUnit(CompilationUnit node) {
var fileName = node.declaredElement.source.shortName;
if (isStrictDartFileName(fileName) &&
!isLowerCaseUnderScoreWithDots(fileName)) {
final fileName = node.declaredElement.source.shortName;
if (!isValidDartFileName(fileName)) {
rule.reportLint(node);
}
}
Expand Down
60 changes: 60 additions & 0 deletions lib/src/util/ascii_utils.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// 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.

/// String utilities that use underlying ASCII codes for improved performance.
/// Ultimately we should consider carefully when we use RegExps where a simple
/// loop would do (and would do so far more performantly).
/// See: https://github.com/dart-lang/linter/issues/1828
library ascii_utils;

import 'package:charcode/ascii.dart';

/// Return `true` if the given [character] is the ASCII '.' character.
bool isDot(int character) => character == $dot;

/// Return `true` if the given [character] is a lowercase ASCII character.
bool isLowerCase(int character) => character >= $a && character <= $z;

/// Return `true` if the given [character] an ASCII number character.
bool isNumber(int character) => character >= 48 && character <= 57;

/// Return `true` if the given [character] is the ASCII '_' character.
bool isUnderScore(int character) => character == $_;

/// Check if the given [name] is a valid Dart filename.
///
/// Files with a strict `.dart` extension are required to use:
/// * `lower_snake_case` and are
/// * limited to valid Dart identifiers
///
/// (Files without a strict `.dart` extension are considered valid.)
bool isValidDartFileName(String name) {
if (name.length < 6 || !name.endsWith('.dart')) {
return true;
}

final length = name.length - 5;
for (int i = 1; i < length - 1; ++i) {
final character = name.codeUnitAt(i);
// Indicates a prefixed suffix (like `.g.dart`) which is considered a
// non-strict Dart filename.
if (isDot(character)) {
return true;
}
}

for (int i = 0; i < length; ++i) {
final character = name.codeUnitAt(i);
if (!isLowerCase(character) && !isUnderScore(character)) {
if (isNumber(character)) {
if (i == 0) {
return false;
}
continue;
}
return false;
}
}
return true;
}
12 changes: 3 additions & 9 deletions lib/src/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,16 @@

import 'ast.dart';

const _dot = '\.';

final _identifier = RegExp(r'^([(_|$)a-zA-Z]+([_a-zA-Z0-9])*)$');

final _lowerCamelCase = RegExp(
r'^(_)*[?$a-z][a-z0-9?$]*(([A-Z][a-z0-9?$]*)|([_][0-9][a-z0-9?$]*))*$');

final _lowerCaseUnderScore = RegExp(r'^([a-z]+([_]?[a-z0-9]+)*)+$');

@Deprecated('Prefer: ascii_utils.isValidFileName')
final _lowerCaseUnderScoreWithDots =
RegExp(r'^[a-z][_a-z0-9]*(\.[a-z][_a-z0-9]*)*$');
RegExp(r'^(_)?[_a-z0-9]*(\.[a-z][_a-z0-9]*)*$');

final _pubspec = RegExp(r'^[_]?pubspec\.yaml$');

Expand Down Expand Up @@ -45,17 +44,12 @@ bool isLowerCaseUnderScore(String id) => _lowerCaseUnderScore.hasMatch(id);

/// Returns `true` if this [id] is `lower_camel_case_with_underscores_or.dots`.
bool isLowerCaseUnderScoreWithDots(String id) =>
// ignore: deprecated_member_use_from_same_package
_lowerCaseUnderScoreWithDots.hasMatch(id);

/// Returns `true` if this [fileName] is a Pubspec file.
bool isPubspecFileName(String fileName) => _pubspec.hasMatch(fileName);

/// Returns true if this is a non-prefixed `.dart extension file.
/// * `foo.dart` => true
/// * `foo.css.dart => false
bool isStrictDartFileName(String fileName) =>
isDartFileName(fileName) && _dot.allMatches(fileName).length == 1;

/// Returns `true` if the given code unit [c] is upper case.
bool isUpperCase(int c) => c >= 0x40 && c <= 0x5A;

Expand Down
1 change: 1 addition & 0 deletions pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ dependencies:
yaml: ^2.1.2

dev_dependencies:
benchmark_harness: ^1.0.0
cli_util: ^0.1.2
dart_style: ^1.3.3
github: ^5.0.0
Expand Down
2 changes: 2 additions & 0 deletions test/all.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import 'package:analyzer/src/lint/io.dart';

import 'ascii_utils_test.dart' as ascii_utils;
import 'ast_test.dart' as ast_test;
import 'engine_test.dart' as engine_test;
import 'formatter_test.dart' as formatter_test;
Expand All @@ -19,6 +20,7 @@ main() {
// Redirect output.
outSink = MockIOSink();

ascii_utils.main();
ast_test.main();
engine_test.main();
formatter_test.main();
Expand Down
72 changes: 72 additions & 0 deletions test/ascii_utils_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// 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.

import 'package:linter/src/util/ascii_utils.dart';
import 'package:test/test.dart';

main() {
group('fileNames', () {
group('good', () {
for (var name in goodFileNames) {
test(name, () {
expect(isValidDartFileName(name), isTrue);
});
}
});

group('bad', () {
for (var name in badFileNames) {
test(name, () {
expect(isValidDartFileName(name), isFalse);
});
}
});
});
}

final badFileNames = [
'Foo.dart',
'fooBar.dart',
'.foo_Bar.dart',
'F_B.dart',
'JS.dart',
'JSON.dart',
];

final goodFileNames = [
// Generated files.
'file-system.g.dart',
'SliderMenu.css.dart',
'_file.dart',
'_file.g.dart',
// Non-strict Dart.
'bwu_server.shared.datastore.some_file',
'foo_bar.baz',
'foo_bar.dart',
'foo_bar.g.dart',
'foo_bar',
'foo.bar',
'foo_bar_baz',
'foo',
'foo_',
'foo.bar_baz.bang',
//See: https://github.com/flutter/flutter/pull/1996
'pointycastle.impl.ec_domain_parameters.gostr3410_2001_cryptopro_a',
'a.b',
'a.b.c',
'p2.src.acme',
//See: https://github.com/dart-lang/linter/issues/1803
'_',
'_f',
'__f',
'___f',
'Foo',
'fooBar.',
'.foo_Bar',
'_.',
'.',
'F_B',
'JS',
'JSON',
];
30 changes: 19 additions & 11 deletions test/utils_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,6 @@ main() {
], isDartFileName, isFalse);
});

group('isStrictDartFileName', () {
testEach(['foo.dart', 'a-b.dart'], isStrictDartFileName, isTrue);
testEach([
'a-b.css.dart',
'foo',
], isStrictDartFileName, isFalse);
});

group('pubspec', () {
testEach(['pubspec.yaml', '_pubspec.yaml'], isPubspecFileName, isTrue);
testEach(['__pubspec.yaml', 'foo.yaml'], isPubspecFileName, isFalse);
Expand Down Expand Up @@ -126,7 +118,7 @@ main() {
testEach(bad, isLowerCaseUnderScore, isFalse);
});

group('qualified lower_case_underscores', () {
group('isLowerCaseUnderScoreWithDots', () {
var good = [
'bwu_server.shared.datastore.some_file',
'foo_bar.baz',
Expand All @@ -140,11 +132,27 @@ main() {
'pointycastle.impl.ec_domain_parameters.gostr3410_2001_cryptopro_a',
'a.b',
'a.b.c',
'p2.src.acme'
'p2.src.acme',
//https://github.com/dart-lang/linter/issues/1803
'_',
'_f',
'__f',
'___f',
'_file.dart',
];
testEach(good, isLowerCaseUnderScoreWithDots, isTrue);

var bad = ['Foo', 'fooBar.', '.foo_Bar', '_f', 'F_B', 'JS', 'JSON'];
var bad = [
'Foo',
'fooBar.',
'.foo_Bar',
'_.',
'.',
'F_B',
'JS',
'JSON',
];

testEach(bad, isLowerCaseUnderScoreWithDots, isFalse);
});

Expand Down
105 changes: 105 additions & 0 deletions tool/benchmark.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
// 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.

import 'package:benchmark_harness/benchmark_harness.dart';
import 'package:linter/src/util/ascii_utils.dart';
import 'package:linter/src/utils.dart';

import '../test/ascii_utils_test.dart' as utils_test;

main() {
FileNameRegexpTestBenchmarkGood().report();
FileNameCharLoopTestBenchmarkGood().report();
FileNameRegexpTestBenchmarkBad().report();
FileNameCharLoopTestBenchmarkBad().report();
AllMatchesBenchmark().report();
DotScanBenchmark().report();
}

final badFileNames = utils_test.badFileNames;

final goodFileNames = utils_test.goodFileNames;

class AllMatchesBenchmark extends BaseBenchmark {
const AllMatchesBenchmark() : super('AllMatches');

@override
void run() {
for (var name in ['foo.dart', 'a-b.dart', 'a-b.css.dart', 'foo']) {
//Test.
'\.'.allMatches(name).length;
}
}
}

class BaseBenchmark extends BenchmarkBase {
const BaseBenchmark(String name) : super(name);

@override
void exercise() {
for (int i = 0; i < 100; i++) {
run();
}
}
}

class DotScanBenchmark extends BaseBenchmark {
const DotScanBenchmark() : super('DotScan');

bool hasOneDot(String name) {
int count = 0;
for (int i = 0; i < name.length; ++i) {
final character = name.codeUnitAt(i);
count += isDot(character) ? 1 : 0;
if (count > 1) {
return false;
}
}
return count == 1;
}

@override
void run() {
// ignore: prefer_foreach
for (var name in ['foo.dart', 'a-b.dart', 'a-b.css.dart', 'foo']) {
hasOneDot(name);
}
}
}

class FileNameCharLoopTestBenchmarkBad extends BenchmarkBase {
const FileNameCharLoopTestBenchmarkBad() : super('Loop:Bad');

@override
void run() {
badFileNames.forEach(isValidDartFileName);
}
}

class FileNameCharLoopTestBenchmarkGood extends BenchmarkBase {
const FileNameCharLoopTestBenchmarkGood() : super('Loop:Good');

@override
void run() {
goodFileNames.forEach(isValidDartFileName);
}
}

class FileNameRegexpTestBenchmarkBad extends BenchmarkBase {
const FileNameRegexpTestBenchmarkBad() : super('Regexp:Bad');

@override
void run() {
badFileNames.forEach(isLowerCaseUnderScoreWithDots);
}
}

class FileNameRegexpTestBenchmarkGood extends BaseBenchmark {
const FileNameRegexpTestBenchmarkGood() : super('Regexp:Good');

@override
void run() {
goodFileNames.forEach(isLowerCaseUnderScoreWithDots);
}
}

0 comments on commit 30d0e92

Please sign in to comment.