From 2ffe31d2eb84b9ea088daac07b44d376e514d48b Mon Sep 17 00:00:00 2001 From: Kevin Moore Date: Wed, 19 Sep 2018 09:03:52 -0700 Subject: [PATCH 1/9] Cleanup .gitignore --- .gitignore | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index d9b7de5..11e5fe7 100644 --- a/.gitignore +++ b/.gitignore @@ -1,13 +1,12 @@ +# Generated by pub .dart_tool/ -.idea/ .packages -.pub -.idea -out -packages pubspec.lock + +# Other generated files benchmark/data/pubspec.link.yaml benchmark/data/pubspec.link.lock benchmark/data/hostname.txt benchmark/data/latest_vm.pb.json benchmark/lib/generated +out From c3b54d19e063c8e43670a2b9b5a2f242ad877580 Mon Sep 17 00:00:00 2001 From: Kevin Moore Date: Wed, 19 Sep 2018 09:04:17 -0700 Subject: [PATCH 2/9] Remove analysis_options.yaml - strong mode is enabled by default now --- analysis_options.yaml | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 analysis_options.yaml diff --git a/analysis_options.yaml b/analysis_options.yaml deleted file mode 100644 index a10d4c5..0000000 --- a/analysis_options.yaml +++ /dev/null @@ -1,2 +0,0 @@ -analyzer: - strong-mode: true From 777d7a5930a0460d87ceb31fabb4fd588b46cb4c Mon Sep 17 00:00:00 2001 From: Kevin Moore Date: Wed, 19 Sep 2018 09:04:35 -0700 Subject: [PATCH 3/9] codereview settings is no longer used --- codereview.settings | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 codereview.settings diff --git a/codereview.settings b/codereview.settings deleted file mode 100644 index 6690e23..0000000 --- a/codereview.settings +++ /dev/null @@ -1,3 +0,0 @@ -CODE_REVIEW_SERVER: https://chromiumcodereview.appspot.com/ -VIEW_VC: https://github.com/dart-lang/dart-protoc-plugin/commit/ -CC_LIST: reviews@dartlang.org From af3dfd62aa157d7e528801dec134438bed250370 Mon Sep 17 00:00:00 2001 From: Kevin Moore Date: Wed, 19 Sep 2018 09:19:19 -0700 Subject: [PATCH 4/9] Use `=` for default values in named arguments --- benchmark/lib/dashboard.dart | 2 +- benchmark/lib/dashboard_model.dart | 2 +- benchmark/lib/suite.dart | 2 +- lib/const_generator.dart | 4 ++-- lib/indenting_writer.dart | 5 +++-- lib/names.dart | 4 ++-- test/file_generator_test.dart | 2 +- 7 files changed, 11 insertions(+), 10 deletions(-) diff --git a/benchmark/lib/dashboard.dart b/benchmark/lib/dashboard.dart index ed3db3c..1a42667 100644 --- a/benchmark/lib/dashboard.dart +++ b/benchmark/lib/dashboard.dart @@ -168,7 +168,7 @@ bool isCompatibleBaseline(pb.Suite suite, pb.Report report) { } Future _loadDataFile(String name, - {bool optional: false, String advice}) async { + {bool optional = false, String advice}) async { try { return await HttpRequest.getString("/data/$name"); } catch (e) { diff --git a/benchmark/lib/dashboard_model.dart b/benchmark/lib/dashboard_model.dart index a7a6581..289379f 100644 --- a/benchmark/lib/dashboard_model.dart +++ b/benchmark/lib/dashboard_model.dart @@ -85,7 +85,7 @@ class Row { final Benchmark benchmark; final pb.Sample baseline; final bool selected; - Row(this.request, this.benchmark, this.baseline, {this.selected: true}); + Row(this.request, this.benchmark, this.baseline, {this.selected = true}); /// Returns the response that should be displayed in this row. pb.Response findResponse(pb.Report r) { diff --git a/benchmark/lib/suite.dart b/benchmark/lib/suite.dart index a42dd6f..76a80fb 100644 --- a/benchmark/lib/suite.dart +++ b/benchmark/lib/suite.dart @@ -15,7 +15,7 @@ import 'generated/benchmark.pb.dart' as pb; /// until they're all done. /// [profiler] if supplied, each request will be profiled once. Iterable runSuite(List requests, - {samplesPerBatch: 1, Profiler profiler}) sync* { + {samplesPerBatch = 1, Profiler profiler}) sync* { // Create a blank report with one response per request. var report = new pb.Report()..status = pb.Status.RUNNING; for (var request in requests) { diff --git a/lib/const_generator.dart b/lib/const_generator.dart index 11b9ced..4408a08 100644 --- a/lib/const_generator.dart +++ b/lib/const_generator.dart @@ -78,7 +78,7 @@ bool _maybeWriteSingleLineString(IndentingWriter out, String val) { } } -void _writeListItems(IndentingWriter out, List val, {bool vertical: false}) { +void _writeListItems(IndentingWriter out, List val, {bool vertical = false}) { bool first = true; for (var item in val) { if (!first && !vertical) { @@ -93,7 +93,7 @@ void _writeListItems(IndentingWriter out, List val, {bool vertical: false}) { } void _writeMapItems(IndentingWriter out, Map val, - {bool vertical: false}) { + {bool vertical = false}) { bool first = true; for (var key in val.keys) { if (!first && !vertical) out.print(", "); diff --git a/lib/indenting_writer.dart b/lib/indenting_writer.dart index fc75c6f..c71889a 100644 --- a/lib/indenting_writer.dart +++ b/lib/indenting_writer.dart @@ -33,14 +33,15 @@ class IndentingWriter { } /// Prints a block of text with the body indented one more level. - void addBlock(String start, String end, void body(), {endWithNewline: true}) { + void addBlock(String start, String end, void body(), + {endWithNewline = true}) { _addBlock(start, end, body, endWithNewline, _indent + ' '); } /// Prints a block of text with an unindented body. /// (For example, for triple quotes.) void addUnindentedBlock(String start, String end, void body(), - {endWithNewline: true}) { + {endWithNewline = true}) { _addBlock(start, end, body, endWithNewline, ''); } diff --git a/lib/names.dart b/lib/names.dart index 905036a..e5dab99 100644 --- a/lib/names.dart +++ b/lib/names.dart @@ -98,7 +98,7 @@ class DartNameOptionException implements Exception { /// /// For a nested message or enum, [parent] should be provided /// with the name of the Dart class for the immediate parent. -String messageOrEnumClassName(String descriptorName, {String parent: ''}) { +String messageOrEnumClassName(String descriptorName, {String parent = ''}) { var name = descriptorName; if (parent != '') { name = '${parent}_${descriptorName}'; @@ -145,7 +145,7 @@ String unusedEnumNames(String name, Set existingNames) { /// Throws [DartNameOptionException] if a field has this option and /// it's set to an invalid name. Map messageFieldNames(DescriptorProto descriptor, - {Iterable reserved: const []}) { + {Iterable reserved = const []}) { var sorted = new List.from(descriptor.field) ..sort((FieldDescriptorProto a, FieldDescriptorProto b) { if (a.number < b.number) return -1; diff --git a/test/file_generator_test.dart b/test/file_generator_test.dart index 7963364..f26d828 100644 --- a/test/file_generator_test.dart +++ b/test/file_generator_test.dart @@ -14,7 +14,7 @@ import 'package:test/test.dart'; import 'golden_file.dart'; FileDescriptorProto buildFileDescriptor( - {phoneNumber: true, topLevelEnum: false}) { + {phoneNumber = true, topLevelEnum = false}) { FileDescriptorProto fd = new FileDescriptorProto()..name = 'test'; if (topLevelEnum) { From 867f722f848f07629ea9d1ebfa09d3f0e210f369 Mon Sep 17 00:00:00 2001 From: Kevin Moore Date: Wed, 19 Sep 2018 09:28:14 -0700 Subject: [PATCH 5/9] use Map/Iterable.isNotEmpty --- lib/const_generator.dart | 4 ++-- lib/enum_generator.dart | 2 +- lib/file_generator.dart | 2 +- lib/protobuf_field.dart | 5 +++-- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/const_generator.dart b/lib/const_generator.dart index 4408a08..62b831b 100644 --- a/lib/const_generator.dart +++ b/lib/const_generator.dart @@ -41,8 +41,8 @@ void writeJsonConst(IndentingWriter out, val) { } bool _nonEmptyListOrMap(x) { - if (x is List && !x.isEmpty) return true; - if (x is Map && !x.isEmpty) return true; + if (x is List && x.isNotEmpty) return true; + if (x is Map && x.isNotEmpty) return true; return false; } diff --git a/lib/enum_generator.dart b/lib/enum_generator.dart index 8bd3022..6fe88db 100644 --- a/lib/enum_generator.dart +++ b/lib/enum_generator.dart @@ -69,7 +69,7 @@ class EnumGenerator extends ProtobufContainer { out.println('static const ${classname} $name = ' "const ${classname}._(${val.number}, '$name');"); } - if (!_aliases.isEmpty) { + if (_aliases.isNotEmpty) { out.println(); for (EnumAlias alias in _aliases) { final name = unusedEnumNames(alias.value.name, reservedNames); diff --git a/lib/file_generator.dart b/lib/file_generator.dart index 783ae92..6dbfb63 100644 --- a/lib/file_generator.dart +++ b/lib/file_generator.dart @@ -208,7 +208,7 @@ class FileGenerator extends ProtobufContainer { // Generate code for extensions defined at top-level using a class // name derived from the file name. - if (!extensionGenerators.isEmpty) { + if (extensionGenerators.isNotEmpty) { // TODO(antonm): do not generate a class. String className = extensionClassName(descriptor); out.addBlock('class $className {', '}\n', () { diff --git a/lib/protobuf_field.dart b/lib/protobuf_field.dart index cfbdbd1..13764ca 100644 --- a/lib/protobuf_field.dart +++ b/lib/protobuf_field.dart @@ -251,9 +251,10 @@ class ProtobufField { case FieldDescriptorProto_Type.TYPE_ENUM: var className = sameProtoFile ? baseType.unprefixed : baseType.prefixed; EnumGenerator gen = baseType.generator; - if (descriptor.hasDefaultValue() && !descriptor.defaultValue.isEmpty) { + if (descriptor.hasDefaultValue() && + descriptor.defaultValue.isNotEmpty) { return '$className.${descriptor.defaultValue}'; - } else if (!gen._canonicalValues.isEmpty) { + } else if (gen._canonicalValues.isNotEmpty) { return '$className.${gen._canonicalValues[0].name}'; } return null; From 306c23ac66380ad723b8bd9ccfd0aaffdeaae317 Mon Sep 17 00:00:00 2001 From: Kevin Moore Date: Wed, 19 Sep 2018 09:29:10 -0700 Subject: [PATCH 6/9] benchmark: populate checkedMode with asserts enabled --- benchmark/lib/report.dart | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/benchmark/lib/report.dart b/benchmark/lib/report.dart index 89b79fc..6d7f258 100644 --- a/benchmark/lib/report.dart +++ b/benchmark/lib/report.dart @@ -36,14 +36,12 @@ pb.Platform createPlatform() { get _isDartVM => !identical(1, 1.0); final bool _checkedMode = () { - bool x = true; - try { - // Trigger an exception if we're in checked mode. - x = "" as dynamic; - return x != ""; // return false; suppress unused variable warning - } catch (e) { + var checked = false; + assert(() { + checked = true; return true; - } + }()); + return checked; }(); /// Given the contents of the pubspec.yaml and pubspec.lock files, From e7a31054a3df83acfa828cc784dcb762dd232dde Mon Sep 17 00:00:00 2001 From: Kevin Moore Date: Wed, 19 Sep 2018 09:30:35 -0700 Subject: [PATCH 7/9] Enable the default set of lints from pkg:pedantic --- analysis_options.yaml | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 analysis_options.yaml diff --git a/analysis_options.yaml b/analysis_options.yaml new file mode 100644 index 0000000..1844c97 --- /dev/null +++ b/analysis_options.yaml @@ -0,0 +1,23 @@ +# Copyright (c) 2018, 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. +# +# Google internally enforced rules. See README.md for more information, +# including a list of lints that are intentionally _not_ enforced. + +linter: + rules: + - avoid_empty_else + - avoid_relative_lib_imports + - avoid_return_types_on_setters + - avoid_types_as_parameter_names + - control_flow_in_finally + - no_duplicate_case_values + - prefer_contains + - prefer_equal_for_default_values + - prefer_is_not_empty + - recursive_getters + - throw_in_finally + - unrelated_type_equality_checks + - use_rethrow_when_possible + - valid_regexps From 6abea8b61f84c96bf48de7a247cbfea662016664 Mon Sep 17 00:00:00 2001 From: Kevin Moore Date: Wed, 19 Sep 2018 09:55:09 -0700 Subject: [PATCH 8/9] Fix up benchmark reporting --- benchmark/lib/report.dart | 84 ++++++++------------------------------- pubspec.yaml | 5 +++ 2 files changed, 22 insertions(+), 67 deletions(-) diff --git a/benchmark/lib/report.dart b/benchmark/lib/report.dart index 6d7f258..38c9169 100644 --- a/benchmark/lib/report.dart +++ b/benchmark/lib/report.dart @@ -6,6 +6,8 @@ library protoc.benchmark.report; import 'dart:convert' show jsonEncode; +import 'package:yaml/yaml.dart'; + import 'generated/benchmark.pb.dart' as pb; pb.Response findUpdatedResponse(pb.Report beforeRep, pb.Report afterRep) { @@ -67,75 +69,23 @@ pb.Packages createPackages(String pubspecYaml, String pubspecLock) { /// - assumes all other lines without a value except 'description' /// start a new package /// - only allows known keys within a package -List _parseLockFile(String contents) { - var out = []; - - bool inPackages = false; - bool inDescription = false; - pb.PackageVersion pv = null; - var lineNumber = 0; - for (var line in contents.split("\n")) { - lineNumber++; - line = line.trim(); // ignore indentation - if (line == "" || line.startsWith("#")) continue; - - // find key and value - var colon = line.indexOf(":"); - if (colon == -1) { - throw "can't parse pubspec.lock at line $lineNumber"; - } - var key = line.substring(0, colon).trim(); - var value = line.substring(colon + 1).trim(); - if (value.length >= 2 && value.startsWith('"') && value.endsWith('"')) { - value = value.substring(1, value.length - 1); - } - - if (!inPackages) { - if (key == "packages" && value == "") { - inPackages = true; - continue; - } - throw "can't parse pubspec.lock at line $lineNumber"; - } - - if (value == "") { - if (key == "description") { - inDescription = true; - continue; - } - if (pv != null) out.add(pv); - pv = new pb.PackageVersion()..name = key; - continue; - } - - if (pv == null) { - throw "can't parse pubspec.lock at line $lineNumber - no value for $key"; - } - - if (inDescription && (key == "name" || key == "url")) continue; - inDescription = false; - - switch (key) { - case "description": - break; - case "source": - pv.source = value; - break; - case "version": - pv.version = value; - break; - case "path": - pv.path = value; - break; - case "relative": - break; // ignore - case "sdk": - break; // ignore - default: - throw "can't parse pubspec.lock at line $lineNumber - unknown key $key"; +Iterable _parseLockFile(String contents) sync* { + var yaml = loadYaml(contents) as YamlMap; + var packages = yaml['packages'] as YamlMap; + + for (var entry in packages.entries) { + var map = entry.value as YamlMap; + var pkgVersion = new pb.PackageVersion() + ..name = entry.key as String + ..source = map['source'] + ..version = map['version']; + + var path = (map['description'] as YamlMap)['path']; + if (path != null) { + pkgVersion.path = path; } + yield pkgVersion; } - return out; } /// Encodes a report as nicely-formatted JSON. diff --git a/pubspec.yaml b/pubspec.yaml index 0ac32e9..feb08ee 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -3,14 +3,19 @@ version: 0.10.2 author: Dart Team description: Protoc compiler plugin to generate Dart code homepage: https://github.com/dart-lang/dart-protoc-plugin + environment: sdk: '>=2.0.0 <3.0.0' + dependencies: fixnum: ^0.10.5 path: ^1.0.0 protobuf: ^0.10.2 dart_style: ^1.0.6 + dev_dependencies: test: ^1.3.0 + yaml: ^2.1.15 + executables: protoc-gen-dart: protoc_plugin From 8244a37169a7f19d038731db331bf751de9fbd94 Mon Sep 17 00:00:00 2001 From: Kevin Moore Date: Wed, 19 Sep 2018 10:16:37 -0700 Subject: [PATCH 9/9] Adding build_* config to enable benchmarks via browser --- benchmark/props.html | 3 +-- benchmark/{props.dart => props_browser.dart} | 0 benchmark/readjson.html | 3 +-- benchmark/{readjson.dart => readjson_browser.dart} | 0 build.yaml | 8 ++++++++ pubspec.yaml | 2 ++ 6 files changed, 12 insertions(+), 4 deletions(-) rename benchmark/{props.dart => props_browser.dart} (100%) rename benchmark/{readjson.dart => readjson_browser.dart} (100%) create mode 100644 build.yaml diff --git a/benchmark/props.html b/benchmark/props.html index 571dc66..82020e4 100644 --- a/benchmark/props.html +++ b/benchmark/props.html @@ -10,7 +10,6 @@ Loading... - - + diff --git a/benchmark/props.dart b/benchmark/props_browser.dart similarity index 100% rename from benchmark/props.dart rename to benchmark/props_browser.dart diff --git a/benchmark/readjson.html b/benchmark/readjson.html index 26095f7..67a9b16 100644 --- a/benchmark/readjson.html +++ b/benchmark/readjson.html @@ -10,7 +10,6 @@ Loading... - - + diff --git a/benchmark/readjson.dart b/benchmark/readjson_browser.dart similarity index 100% rename from benchmark/readjson.dart rename to benchmark/readjson_browser.dart diff --git a/build.yaml b/build.yaml new file mode 100644 index 0000000..b26ad9f --- /dev/null +++ b/build.yaml @@ -0,0 +1,8 @@ +# Configuration for Dart build system - https://github.com/dart-lang/build +# Enables running benchmarks via browser: `pub run build_runner serve` +targets: + $default: + builders: + build_web_compilers|entrypoint: + generate_for: + - benchmark/*_browser.dart diff --git a/pubspec.yaml b/pubspec.yaml index feb08ee..569cff6 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -14,6 +14,8 @@ dependencies: dart_style: ^1.0.6 dev_dependencies: + build_runner: ^0.10.0 + build_web_compilers: ^0.4.0 test: ^1.3.0 yaml: ^2.1.15