Skip to content

Commit 75a6d0d

Browse files
authored
Remove redundant PubSummary.lockedVersion (dart-lang#28)
Remove redundant PubSummary.lockedVersion But keep the logic around to validate the pub output parsing
1 parent 903377e commit 75a6d0d

File tree

4 files changed

+37
-76
lines changed

4 files changed

+37
-76
lines changed

lib/pana.dart

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,18 @@ class PackageAnalyzer {
7979

8080
PubSummary summary;
8181
if (upgrade.exitCode == 0) {
82-
summary = PubSummary.create(upgrade.stdout, path: pkgDir);
83-
log.info("Package version: ${summary.pkgVersion}");
82+
try {
83+
summary = PubSummary.create(upgrade.stdout, path: pkgDir);
84+
} catch (e, stack) {
85+
log.severe("Problem summarizing package", e, stack);
86+
//(TODO)kevmoo - should add a helper that handles logging exceptions
87+
// and writing to issues in one go.
88+
issues.add(new AnalyzerIssue(
89+
AnalyzerScopes.pubspec, "Problem summarizing package: $e"));
90+
}
91+
if (summary != null) {
92+
log.info("Package version: ${summary.pkgVersion}");
93+
}
8494
} else {
8595
String message;
8696
if (upgrade.exitCode > 0) {

lib/src/pub_summary.dart

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,13 @@ import 'utils.dart';
1313

1414
class PubSummary {
1515
static final _solvePkgLine = new RegExp(
16-
r"(?:[><\+ ]) (\w+) (\S+)(?: \((\S+) available\))?(?: from .+)?");
16+
r"(?:[><\+\! ]) (\w+) (\S+)(?: \((\S+) available\))?(?: from .+)?");
1717

1818
final Map<String, Version> packageVersions;
1919
final Map<String, Version> availableVersions;
20-
final Map<String, Version> lockedVersions;
2120
final Map<String, Object> pubspec;
2221

23-
PubSummary._(this.packageVersions, this.availableVersions,
24-
this.lockedVersions, this.pubspec);
22+
PubSummary._(this.packageVersions, this.availableVersions, this.pubspec);
2523

2624
static PubSummary create(String procStdout, {String path}) {
2725
var pkgVersions = <String, Version>{};
@@ -53,7 +51,6 @@ class PubSummary {
5351
// it's empty – which is fine for a package with no dependencies
5452
}
5553

56-
Map<String, Version> lockedVersions;
5754
Map<String, Object> pubspecContent;
5855

5956
if (path != null) {
@@ -66,26 +63,42 @@ class PubSummary {
6663
Map lockMap = yamlToJson(lockFileContent);
6764
Map pkgs = lockMap['packages'];
6865
if (pkgs != null) {
69-
lockedVersions = {};
66+
var expectedPackages = pkgVersions.keys.toSet();
67+
7068
pkgs.forEach((String key, Map m) {
71-
lockedVersions[key] = new Version.parse(m['version']);
69+
if (!expectedPackages.remove(key)) {
70+
throw new StateError(
71+
"Did not parse package `$key` from pub output, "
72+
"but it was found in `pubspec.lock`.");
73+
}
74+
75+
var lockedVersion = new Version.parse(m['version']);
76+
if (pkgVersions[key] != lockedVersion) {
77+
throw new StateError(
78+
"For $key, the parsed version ${pkgVersions[key]} did not "
79+
"match the locked version $lockedVersion.");
80+
}
7281
});
82+
83+
if (expectedPackages.isNotEmpty) {
84+
throw new StateError(
85+
"We parsed more packaged than were found in the lock file: "
86+
"${expectedPackages.join(', ')}");
87+
}
7388
}
7489
}
7590
}
7691
}
7792

78-
return new PubSummary._(
79-
pkgVersions, availVersions, lockedVersions, pubspecContent);
93+
return new PubSummary._(pkgVersions, availVersions, pubspecContent);
8094
}
8195

8296
factory PubSummary.fromJson(Map<String, dynamic> json) {
8397
var packageVersions = _jsonMapToVersion(json['packages']);
8498
var availableVersions = _jsonMapToVersion(json['availablePackages']);
85-
var lockedVersions = _jsonMapToVersion(json['lockedVersions']);
8699

87-
return new PubSummary._(packageVersions, availableVersions, lockedVersions,
88-
json['pubspecContent']);
100+
return new PubSummary._(
101+
packageVersions, availableVersions, json['pubspecContent']);
89102
}
90103

91104
Map<String, int> getStats() {
@@ -190,7 +203,6 @@ class PubSummary {
190203
Map<String, dynamic> toJson() => <String, dynamic>{
191204
'pubspecContent': pubspec,
192205
'packages': _versionMapToJson(packageVersions),
193-
'lockedVersions': _versionMapToJson(lockedVersions),
194206
'availablePackages': _versionMapToJson(availableVersions),
195207
};
196208

test/end2end/http_data.dart

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,18 +37,6 @@ final data = {
3737
"typed_data": "1.1.3",
3838
"unittest": "0.11.7"
3939
},
40-
"lockedVersions": {
41-
"async": "1.13.3",
42-
"charcode": "1.1.1",
43-
"collection": "1.14.1",
44-
"http_parser": "3.1.1",
45-
"path": startsWith("1.4."),
46-
"source_span": "1.4.0",
47-
"stack_trace": "1.7.3",
48-
"string_scanner": "1.0.2",
49-
"typed_data": "1.1.3",
50-
"unittest": "0.11.7"
51-
},
5240
"availablePackages": {"unittest": "0.12.4+1"}
5341
},
5442
"dartFiles": {

test/end2end/pub_server_data.dart

Lines changed: 0 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -59,55 +59,6 @@ final data = {
5959
"yaml": "2.1.12"
6060
},
6161
"availablePackages": {},
62-
"lockedVersions": {
63-
"analyzer": "0.30.0+2",
64-
"archive": "1.0.29",
65-
"args": "0.13.7",
66-
"async": "1.13.3",
67-
"barback": "0.15.2+11",
68-
"boolean_selector": "1.0.2",
69-
"charcode": "1.1.1",
70-
"cli_util": "0.1.1",
71-
"collection": "1.14.1",
72-
"convert": "2.0.1",
73-
"crypto": "2.0.1",
74-
"csslib": "0.14.0",
75-
"front_end": "0.1.0-alpha.4",
76-
"glob": "1.1.3",
77-
"html": "0.13.2",
78-
"http": "0.11.3+13",
79-
"http_multi_server": "2.0.3",
80-
"http_parser": "3.1.1",
81-
"isolate": "1.0.0",
82-
"kernel": "0.3.0-alpha.1",
83-
"logging": "0.11.3+1",
84-
"matcher": "0.12.1+1",
85-
"meta": "1.0.5",
86-
"mime": "0.9.3",
87-
"package_config": "1.0.1",
88-
"package_resolver": "1.0.2",
89-
"path": startsWith("1.4."),
90-
"plugin": "0.2.0",
91-
"pool": "1.3.1",
92-
"pub_semver": "1.3.2",
93-
"shelf": "0.6.7+2",
94-
"shelf_packages_handler": "1.0.0",
95-
"shelf_static": "0.2.4",
96-
"shelf_web_socket": "0.2.1",
97-
"source_map_stack_trace": "1.1.4",
98-
"source_maps": "0.10.4",
99-
"source_span": "1.4.0",
100-
"stack_trace": "1.7.3",
101-
"stream_channel": "1.6.1",
102-
"string_scanner": "1.0.2",
103-
"term_glyph": "1.0.0",
104-
"test": startsWith("0.12."),
105-
"typed_data": "1.1.3",
106-
"utf": "0.9.0+3",
107-
"watcher": "0.9.7+3",
108-
"web_socket_channel": "1.0.4",
109-
"yaml": "2.1.12"
110-
},
11162
"pubspecContent": {
11263
"author": "Dart Team <[email protected]>",
11364
"dependencies": {

0 commit comments

Comments
 (0)