Skip to content
This repository was archived by the owner on Jan 26, 2021. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -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
25 changes: 23 additions & 2 deletions analysis_options.yaml
Original file line number Diff line number Diff line change
@@ -1,2 +1,23 @@
analyzer:
strong-mode: true
# 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
2 changes: 1 addition & 1 deletion benchmark/lib/dashboard.dart
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ bool isCompatibleBaseline(pb.Suite suite, pb.Report report) {
}

Future<String> _loadDataFile(String name,
{bool optional: false, String advice}) async {
{bool optional = false, String advice}) async {
try {
return await HttpRequest.getString("/data/$name");
} catch (e) {
Expand Down
2 changes: 1 addition & 1 deletion benchmark/lib/dashboard_model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
96 changes: 22 additions & 74 deletions benchmark/lib/report.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -36,14 +38,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(() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not checking the same thing.

The code as originally written:

  • in Dart 1 was checking for --checked.
  • in Dart 2 was checking for --omit-implicit-checks when compiled with dart2js

Now it is checking for --enable-asserts.

So I expect a performance regression on 'production' apps.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rakudrama – suggestions?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert this part

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And document it better.

checked = true;
return true;
}
}());
return checked;
}();

/// Given the contents of the pubspec.yaml and pubspec.lock files,
Expand All @@ -69,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<pb.PackageVersion> _parseLockFile(String contents) {
var out = <pb.PackageVersion>[];

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<pb.PackageVersion> _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.
Expand Down
2 changes: 1 addition & 1 deletion benchmark/lib/suite.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<pb.Report> runSuite(List<pb.Request> 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) {
Expand Down
3 changes: 1 addition & 2 deletions benchmark/props.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
Loading...
</div>

<script type="application/dart" src="props.dart"></script>
<script src="packages/browser/dart.js"></script>
<script defer src="props_browser.dart.js"></script>
</body>
</html>
File renamed without changes.
3 changes: 1 addition & 2 deletions benchmark/readjson.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
Loading...
</div>

<script type="application/dart" src="readjson.dart"></script>
<script src="packages/browser/dart.js"></script>
<script defer src="readjson_browser.dart.js"></script>
</body>
</html>
File renamed without changes.
8 changes: 8 additions & 0 deletions build.yaml
Original file line number Diff line number Diff line change
@@ -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
3 changes: 0 additions & 3 deletions codereview.settings

This file was deleted.

8 changes: 4 additions & 4 deletions lib/const_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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) {
Expand All @@ -93,7 +93,7 @@ void _writeListItems(IndentingWriter out, List val, {bool vertical: false}) {
}

void _writeMapItems(IndentingWriter out, Map<dynamic, dynamic> val,
{bool vertical: false}) {
{bool vertical = false}) {
bool first = true;
for (var key in val.keys) {
if (!first && !vertical) out.print(", ");
Expand Down
2 changes: 1 addition & 1 deletion lib/enum_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion lib/file_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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', () {
Expand Down
5 changes: 3 additions & 2 deletions lib/indenting_writer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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, '');
}

Expand Down
4 changes: 2 additions & 2 deletions lib/names.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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}';
Expand Down Expand Up @@ -145,7 +145,7 @@ String unusedEnumNames(String name, Set<String> existingNames) {
/// Throws [DartNameOptionException] if a field has this option and
/// it's set to an invalid name.
Map<String, MemberNames> messageFieldNames(DescriptorProto descriptor,
{Iterable<String> reserved: const []}) {
{Iterable<String> reserved = const []}) {
var sorted = new List<FieldDescriptorProto>.from(descriptor.field)
..sort((FieldDescriptorProto a, FieldDescriptorProto b) {
if (a.number < b.number) return -1;
Expand Down
5 changes: 3 additions & 2 deletions lib/protobuf_field.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 7 additions & 0 deletions pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,21 @@ version: 0.10.2
author: Dart Team <[email protected]>
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:
build_runner: ^0.10.0
build_web_compilers: ^0.4.0
test: ^1.3.0
yaml: ^2.1.15

executables:
protoc-gen-dart: protoc_plugin
2 changes: 1 addition & 1 deletion test/file_generator_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down