From 275cf5e31a0d9b33ab2d3adce13c8b5b515cbcab Mon Sep 17 00:00:00 2001 From: Nate Bosch <nbosch1@gmail.com> Date: Thu, 26 Jul 2018 13:47:59 -0700 Subject: [PATCH] Fix some style nits in parse_metadata and test (#917) - Use `??`, `??=`, and `?.` instead of explicit `== null`. - Use consistent single quotes --- lib/src/runner/parse_metadata.dart | 41 +++++----- test/runner/parse_metadata_test.dart | 110 +++++++++++++-------------- 2 files changed, 74 insertions(+), 77 deletions(-) diff --git a/lib/src/runner/parse_metadata.dart b/lib/src/runner/parse_metadata.dart index 5da4ea76..eeff2f63 100644 --- a/lib/src/runner/parse_metadata.dart +++ b/lib/src/runner/parse_metadata.dart @@ -51,8 +51,7 @@ class _Parser { _prefixes = directives .map((directive) { if (directive is ImportDirective) { - if (directive.prefix == null) return null; - return directive.prefix.name; + return directive.prefix?.name; } else { return null; } @@ -184,8 +183,8 @@ class _Parser { if (name.contains(anchoredHyphenatedIdentifier)) return name; throw new SourceSpanFormatException( - "Invalid tag name. Tags must be (optionally hyphenated) Dart " - "identifiers.", + 'Invalid tag name. Tags must be (optionally hyphenated) Dart ' + 'identifiers.', _spanFor(tagExpression)); }).toSet(); } @@ -257,14 +256,12 @@ class _Parser { .map((key, value) => new MapEntry(key, _parseInt(value))); return new Duration( - days: values["days"] == null ? 0 : values["days"], - hours: values["hours"] == null ? 0 : values["hours"], - minutes: values["minutes"] == null ? 0 : values["minutes"], - seconds: values["seconds"] == null ? 0 : values["seconds"], - milliseconds: - values["milliseconds"] == null ? 0 : values["milliseconds"], - microseconds: - values["microseconds"] == null ? 0 : values["microseconds"]); + days: values['days'] ?? 0, + hours: values['hours'] ?? 0, + minutes: values['minutes'] ?? 0, + seconds: values['seconds'] ?? 0, + milliseconds: values['milliseconds'] ?? 0, + microseconds: values['microseconds'] ?? 0); } Map<String, Expression> _parseNamedArguments(ArgumentList arguments) => @@ -280,7 +277,7 @@ class _Parser { void _assertSingle(Object existing, String name, AstNode node) { if (existing == null) return; throw new SourceSpanFormatException( - "Only a single $name may be used.", _spanFor(node)); + 'Only a single $name may be used.', _spanFor(node)); } /// Resolves a constructor name from its type [identifier] and its @@ -321,7 +318,7 @@ class _Parser { String _parseConstructor(Expression expression, String className) { if (expression is! InstanceCreationExpression) { throw new SourceSpanFormatException( - "Expected a $className.", _spanFor(expression)); + 'Expected a $className.', _spanFor(expression)); } var constructor = expression as InstanceCreationExpression; @@ -332,7 +329,7 @@ class _Parser { if (actualClassName != className) { throw new SourceSpanFormatException( - "Expected a $className.", _spanFor(constructor)); + 'Expected a $className.', _spanFor(constructor)); } return constructorName; @@ -344,12 +341,12 @@ class _Parser { /// with the [key] and [value] parameters. Map<K, V> _parseMap<K, V>(Expression expression, {K key(Expression expression), V value(Expression expression)}) { - if (key == null) key = (expression) => expression as K; - if (value == null) value = (expression) => expression as V; + key ??= (expression) => expression as K; + value ??= (expression) => expression as V; if (expression is! MapLiteral) { throw new SourceSpanFormatException( - "Expected a Map.", _spanFor(expression)); + 'Expected a Map.', _spanFor(expression)); } var map = expression as MapLiteral; @@ -362,7 +359,7 @@ class _Parser { List<Expression> _parseList(Expression expression) { if (expression is! ListLiteral) { throw new SourceSpanFormatException( - "Expected a List.", _spanFor(expression)); + 'Expected a List.', _spanFor(expression)); } var list = expression as ListLiteral; @@ -375,21 +372,21 @@ class _Parser { if (expression is IntegerLiteral) return expression.value; if (expression is DoubleLiteral) return expression.value; throw new SourceSpanFormatException( - "Expected a number.", _spanFor(expression)); + 'Expected a number.', _spanFor(expression)); } /// Parses a constant int literal. int _parseInt(Expression expression) { if (expression is IntegerLiteral) return expression.value; throw new SourceSpanFormatException( - "Expected an integer.", _spanFor(expression)); + 'Expected an integer.', _spanFor(expression)); } /// Parses a constant String literal. StringLiteral _parseString(Expression expression) { if (expression is StringLiteral) return expression; throw new SourceSpanFormatException( - "Expected a String.", _spanFor(expression)); + 'Expected a String.', _spanFor(expression)); } /// Creates a [SourceSpan] for [node]. diff --git a/test/runner/parse_metadata_test.dart b/test/runner/parse_metadata_test.dart index acb86102..c87e09f1 100644 --- a/test/runner/parse_metadata_test.dart +++ b/test/runner/parse_metadata_test.dart @@ -2,7 +2,7 @@ // 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. -@TestOn("vm") +@TestOn('vm') import 'dart:io'; import 'package:path/path.dart' as p; @@ -19,27 +19,27 @@ String _path; void main() { setUp(() { _sandbox = createTempDir(); - _path = p.join(_sandbox, "test.dart"); + _path = p.join(_sandbox, 'test.dart'); }); tearDown(() { new Directory(_sandbox).deleteSync(recursive: true); }); - test("returns empty metadata for an empty file", () { - new File(_path).writeAsStringSync(""); + test('returns empty metadata for an empty file', () { + new File(_path).writeAsStringSync(''); var metadata = parseMetadata(_path, new Set()); expect(metadata.testOn, equals(PlatformSelector.all)); expect(metadata.timeout.scaleFactor, equals(1)); }); - test("ignores irrelevant annotations", () { - new File(_path).writeAsStringSync("@Fblthp\n@Fblthp.foo\nlibrary foo;"); + test('ignores irrelevant annotations', () { + new File(_path).writeAsStringSync('@Fblthp\n@Fblthp.foo\nlibrary foo;'); var metadata = parseMetadata(_path, new Set()); expect(metadata.testOn, equals(PlatformSelector.all)); }); - test("parses a prefixed annotation", () { + test('parses a prefixed annotation', () { new File(_path).writeAsStringSync("@foo.TestOn('vm')\n" "import 'package:test/test.dart' as foo;"); var metadata = parseMetadata(_path, new Set()); @@ -48,8 +48,8 @@ void main() { metadata.testOn.evaluate(new SuitePlatform(Runtime.chrome)), isFalse); }); - group("@TestOn:", () { - test("parses a valid annotation", () { + group('@TestOn:', () { + test('parses a valid annotation', () { new File(_path).writeAsStringSync("@TestOn('vm')\nlibrary foo;"); var metadata = parseMetadata(_path, new Set()); expect(metadata.testOn.evaluate(new SuitePlatform(Runtime.vm)), isTrue); @@ -57,14 +57,14 @@ void main() { metadata.testOn.evaluate(new SuitePlatform(Runtime.chrome)), isFalse); }); - test("ignores a constructor named TestOn", () { + test('ignores a constructor named TestOn', () { new File(_path).writeAsStringSync("@foo.TestOn('foo')\nlibrary foo;"); var metadata = parseMetadata(_path, new Set()); expect(metadata.testOn, equals(PlatformSelector.all)); }); - group("throws an error for", () { - test("multiple @TestOns", () { + group('throws an error for', () { + test('multiple @TestOns', () { new File(_path) .writeAsStringSync("@TestOn('foo')\n@TestOn('bar')\nlibrary foo;"); expect(() => parseMetadata(_path, new Set()), throwsFormatException); @@ -72,9 +72,9 @@ void main() { }); }); - group("@Timeout:", () { - test("parses a valid duration annotation", () { - new File(_path).writeAsStringSync(""" + group('@Timeout:', () { + test('parses a valid duration annotation', () { + new File(_path).writeAsStringSync(''' @Timeout(const Duration( hours: 1, minutes: 2, @@ -83,7 +83,7 @@ void main() { microseconds: 5)) library foo; -"""); +'''); var metadata = parseMetadata(_path, new Set()); expect( metadata.timeout.duration, @@ -95,8 +95,8 @@ library foo; microseconds: 5))); }); - test("parses a valid duration annotation omitting const", () { - new File(_path).writeAsStringSync(""" + test('parses a valid duration annotation omitting const', () { + new File(_path).writeAsStringSync(''' @Timeout(Duration( hours: 1, minutes: 2, @@ -105,7 +105,7 @@ library foo; microseconds: 5)) library foo; -"""); +'''); var metadata = parseMetadata(_path, new Set()); expect( metadata.timeout.duration, @@ -117,74 +117,74 @@ library foo; microseconds: 5))); }, skip: 'https://github.com/dart-lang/test/issues/915'); - test("parses a valid int factor annotation", () { - new File(_path).writeAsStringSync(""" + test('parses a valid int factor annotation', () { + new File(_path).writeAsStringSync(''' @Timeout.factor(1) library foo; -"""); +'''); var metadata = parseMetadata(_path, new Set()); expect(metadata.timeout.scaleFactor, equals(1)); }); - test("parses a valid double factor annotation", () { - new File(_path).writeAsStringSync(""" + test('parses a valid double factor annotation', () { + new File(_path).writeAsStringSync(''' @Timeout.factor(0.5) library foo; -"""); +'''); var metadata = parseMetadata(_path, new Set()); expect(metadata.timeout.scaleFactor, equals(0.5)); }); - test("parses a valid Timeout.none annotation", () { - new File(_path).writeAsStringSync(""" + test('parses a valid Timeout.none annotation', () { + new File(_path).writeAsStringSync(''' @Timeout.none library foo; -"""); +'''); var metadata = parseMetadata(_path, new Set()); expect(metadata.timeout, same(Timeout.none)); }); - test("ignores a constructor named Timeout", () { + test('ignores a constructor named Timeout', () { new File(_path).writeAsStringSync("@foo.Timeout('foo')\nlibrary foo;"); var metadata = parseMetadata(_path, new Set()); expect(metadata.timeout.scaleFactor, equals(1)); }); - group("throws an error for", () { - test("multiple @Timeouts", () { + group('throws an error for', () { + test('multiple @Timeouts', () { new File(_path).writeAsStringSync( - "@Timeout.factor(1)\n@Timeout.factor(2)\nlibrary foo;"); + '@Timeout.factor(1)\n@Timeout.factor(2)\nlibrary foo;'); expect(() => parseMetadata(_path, new Set()), throwsFormatException); }); }); }); - group("@Skip:", () { - test("parses a valid annotation", () { - new File(_path).writeAsStringSync("@Skip()\nlibrary foo;"); + group('@Skip:', () { + test('parses a valid annotation', () { + new File(_path).writeAsStringSync('@Skip()\nlibrary foo;'); var metadata = parseMetadata(_path, new Set()); expect(metadata.skip, isTrue); expect(metadata.skipReason, isNull); }); - test("parses a valid annotation with a reason", () { + test('parses a valid annotation with a reason', () { new File(_path).writeAsStringSync("@Skip('reason')\nlibrary foo;"); var metadata = parseMetadata(_path, new Set()); expect(metadata.skip, isTrue); expect(metadata.skipReason, equals('reason')); }); - test("ignores a constructor named Skip", () { + test('ignores a constructor named Skip', () { new File(_path).writeAsStringSync("@foo.Skip('foo')\nlibrary foo;"); var metadata = parseMetadata(_path, new Set()); expect(metadata.skip, isFalse); }); - group("throws an error for", () { - test("multiple @Skips", () { + group('throws an error for', () { + test('multiple @Skips', () { new File(_path) .writeAsStringSync("@Skip('foo')\n@Skip('bar')\nlibrary foo;"); expect(() => parseMetadata(_path, new Set()), throwsFormatException); @@ -192,21 +192,21 @@ library foo; }); }); - group("@Tags:", () { - test("parses a valid annotation", () { + group('@Tags:', () { + test('parses a valid annotation', () { new File(_path).writeAsStringSync("@Tags(['a'])\nlibrary foo;"); var metadata = parseMetadata(_path, new Set()); - expect(metadata.tags, equals(["a"])); + expect(metadata.tags, equals(['a'])); }); - test("ignores a constructor named Tags", () { + test('ignores a constructor named Tags', () { new File(_path).writeAsStringSync("@foo.Tags(['a'])\nlibrary foo;"); var metadata = parseMetadata(_path, new Set()); expect(metadata.tags, isEmpty); }); - group("throws an error for", () { - test("multiple @Tags", () { + group('throws an error for', () { + test('multiple @Tags', () { new File(_path) .writeAsStringSync("@Tags(['a'])\n@Tags(['b'])\nlibrary foo;"); expect(() => parseMetadata(_path, new Set()), throwsFormatException); @@ -214,14 +214,14 @@ library foo; }); }); - group("@OnPlatform:", () { - test("parses a valid annotation", () { - new File(_path).writeAsStringSync(""" + group('@OnPlatform:', () { + test('parses a valid annotation', () { + new File(_path).writeAsStringSync(''' @OnPlatform({ 'chrome': const Timeout.factor(2), 'vm': [const Skip(), const Timeout.factor(3)] }) -library foo;"""); +library foo;'''); var metadata = parseMetadata(_path, new Set()); var key = metadata.onPlatform.keys.first; @@ -238,14 +238,14 @@ library foo;"""); expect(value.timeout.scaleFactor, equals(3)); }); - test("ignores a constructor named OnPlatform", () { + test('ignores a constructor named OnPlatform', () { new File(_path).writeAsStringSync("@foo.OnPlatform('foo')\nlibrary foo;"); var metadata = parseMetadata(_path, new Set()); expect(metadata.testOn, equals(PlatformSelector.all)); }); - group("throws an error for", () { - test("a map with a unparseable key", () { + group('throws an error for', () { + test('a map with a unparseable key', () { new File(_path).writeAsStringSync( "@OnPlatform({'invalid': Skip()})\nlibrary foo;"); expect(() => parseMetadata(_path, new Set()), throwsFormatException); @@ -257,15 +257,15 @@ library foo;"""); expect(() => parseMetadata(_path, new Set()), throwsFormatException); }); - test("a map with an invalid value in a list", () { + test('a map with an invalid value in a list', () { new File(_path).writeAsStringSync( "@OnPlatform({'vm': [const TestOn('vm')]})\nlibrary foo;"); expect(() => parseMetadata(_path, new Set()), throwsFormatException); }); - test("multiple @OnPlatforms", () { + test('multiple @OnPlatforms', () { new File(_path).writeAsStringSync( - "@OnPlatform({})\n@OnPlatform({})\nlibrary foo;"); + '@OnPlatform({})\n@OnPlatform({})\nlibrary foo;'); expect(() => parseMetadata(_path, new Set()), throwsFormatException); }); }); -- GitLab