Skip to content
Closed
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
171 changes: 171 additions & 0 deletions app/lib/analyzer.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
// Copyright (c) 2017, 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.

library pub_dartlang_org.analysis;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe use a consistent name for the filename and library name (i.e. analysis -> analyzer).


import 'dart:async';
import 'dart:io';

import 'package:yaml/yaml.dart' as yaml;

import 'model_properties.dart';
import 'utils.dart';

/// Collects the preparation, analysis and cleanup functions for analyzing a
/// package.
class Analyzer {
final File _sourceTarGz;
final Directory _workDir;
final bool _clearWorkDir;
Directory _packageDir;

Pubspec pubspec;
PubspecLock pubspecLock;

Analyzer._(this._sourceTarGz, this._workDir, this._clearWorkDir);

factory Analyzer(String source, {String workDir}) {
final Directory wd = workDir != null
? new Directory(workDir)
: Directory.systemTemp.createTempSync();
return new Analyzer._(new File(source), wd, workDir == null);
}
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it would be easier to read when using only one instead of two constructors:

Analyzer(String source, {String workdir}) 
    : _sourceTarGz = new File(source),
      _workDir  = workdir != null ? new Directory(workdir) : Directory.systemTemp.createTempSync(),
      _clearWorkDir = workdir == null;


/// Prepares the directories for the analysis and extracts the package.
Future setUp() async {
Copy link
Member

Choose a reason for hiding this comment

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

Consider renaming it from setUp to extractTarball.

if (!_sourceTarGz.existsSync()) {
throw new Exception('Source file not found: ${_sourceTarGz.path}');
}
if (!_workDir.existsSync()) {
await _workDir.create(recursive: true);
}
_packageDir = await _workDir.createTemp('package-');
await extractTarball(_sourceTarGz.path, _packageDir.path);
}

Future analyzeAll() async {
await runPubGet();
Copy link
Member

Choose a reason for hiding this comment

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

I have all of this written already - https://github.com/kevmoo/pana.dart

Mostly. Need to expose some of the private impl stuff I have

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

looks good! Any ETA for publishing the recent changes?

Copy link
Member

Choose a reason for hiding this comment

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

Just did...

Copy link
Member

Choose a reason for hiding this comment

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

@kevmoo No you did not! The last upload to package:pana was in 2015!

Copy link
Member

Choose a reason for hiding this comment

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

Also if we want to use package:pana for the task, why this code here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This code precedes my awareness of pana. I think pana is a better place for most of this analysis, and I'm planning to contribute to it (e.g. dart-lang/pana#3). At the same time as soon as it is 'good enough', I'll update this PR with the 'pana' version.

await readPubspecs();
await runDartAnalyzer();
await runDartFormatter();
}

/// Runs pub get in the package directory to fetch the dependencies.
Future runPubGet() async {
// TODO: control the version of Dart SDK
// TODO: control the cache location
// TODO: add timeout handling
final ProcessResult pr =
await Process.run('pub', ['get'], workingDirectory: _packageDir.path);
if (pr.exitCode != 0) {
throw new Exception('pub get error: ${pr.stdout} ${pr.stderr}');
}
}

/// Reads pubspec.yaml and pubspec.lock.
Future readPubspecs() async {
final String pubspecContent = await new File(
_packageDir.path + Platform.pathSeparator + 'pubspec.yaml')
Copy link
Member

Choose a reason for hiding this comment

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

Consider using package:path/path.dart and do path.join(_packagesDir.path, 'pubspec.yaml')

.readAsString();
pubspec = new Pubspec.fromYaml(pubspecContent);
final String pubspecLockContent = await new File(
_packageDir.path + Platform.pathSeparator + 'pubspec.lock')
.readAsString();
pubspecLock = new PubspecLock.fromYaml(pubspecLockContent);
}

Future runDartAnalyzer() async {
// TODO: implement
}

Future runDartFormatter() async {
// TODO: implement
}

void createReport() {
// TODO: implement
}

/// Deletes all directories and files that were created during the analysis.
Future tearDown() async {
if (_clearWorkDir) {
await _workDir?.delete(recursive: true);
} else {
await _packageDir?.delete(recursive: true);
}
}
}

class PubspecLock {
Map _content;
Copy link
Member

Choose a reason for hiding this comment

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

Since you initialize it in the constructor, you might as well make it final (and use initializer rather than constructor body)

List<LockedDependency> _packages;

PubspecLock.fromYaml(String yamlContent) {
_content = yaml.loadYaml(yamlContent);
}

List<LockedDependency> get packages {
if (_packages == null) {
Copy link
Member

Choose a reason for hiding this comment

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

Why do this lazily? The only reason one would construct a PubspecLock object is to call packages on it.

_packages = [];
final Map pkgs = _content['packages'];
if (pkgs != null) {
pkgs.forEach((String key, Map m) {
_packages.add(new LockedDependency.fromMap(key, m));
});
}
_packages.sort((a, b) => a.key.compareTo(b.key));
}
return _packages;
}
}

class LockedDependency {
final String key;
final String descriptionName;
final String descriptionUrl;
final String source;
final String version;

LockedDependency(
{this.key,
this.descriptionName,
this.descriptionUrl,
this.source,
this.version});

factory LockedDependency.fromMap(String key, Map map) {
final Map description = map['description'] ?? {};
return new LockedDependency(
key: key,
descriptionName: description['name'],
descriptionUrl: description['url'],
source: map['source'],
version: map['version']);
}

@override
bool operator ==(Object other) =>
identical(this, other) ||
other is LockedDependency &&
runtimeType == other.runtimeType &&
key == other.key &&
descriptionName == other.descriptionName &&
descriptionUrl == other.descriptionUrl &&
source == other.source &&
version == other.version;

@override
int get hashCode =>
key.hashCode ^
descriptionName.hashCode ^
descriptionUrl.hashCode ^
source.hashCode ^
version.hashCode;

@override
String toString() {
return 'LockedDependency{key: $key, descriptionName: $descriptionName, '
'descriptionUrl: $descriptionUrl, source: $source, version: $version}';
}
}
12 changes: 12 additions & 0 deletions app/lib/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,18 @@ Future<String> readTarballFile(String path, String name) async {
return result.stdout;
}

Future extractTarball(String file, String destinationDir) async {
final args = ['-xzf', file, '-C', destinationDir];
final result = await Process.run('tar', args);
if (result.exitCode != 0) {
_logger.warning('The "tar $args" command failed:\n'
'with exit code: ${result.exitCode}\n'
'stdout: ${result.stdout}\n'
'stderr: ${result.stderr}');
throw new Exception('Failed to extract tarball.');
Copy link
Member

Choose a reason for hiding this comment

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

Consider changing readTarballFile to also log a warning - for consistency.

}
}

String canonicalizeVersion(String version) {
// NOTE: This is a hack because [semver.Version.parse] does not remove
// leading zeros for integer fields.
Expand Down
Binary file added app/test/analyzer_assets/async-1.13.3.tar.gz
Binary file not shown.
59 changes: 59 additions & 0 deletions app/test/analyzer_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// Copyright (c) 2017, 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 'dart:io';

import 'package:test/test.dart';

import 'package:pub_dartlang_org/analyzer.dart';

void main() {
group('Analysis', () {
group('async-1.13.3', () {
Analyzer analyzer;

setUpAll(() async {
analyzer = new Analyzer('test/analyzer_assets/async-1.13.3.tar.gz');
await analyzer.setUp();
await analyzer.analyzeAll();
});

tearDownAll(() async {
await analyzer?.tearDown();
});

test('pubspec.yaml', () async {
expect(analyzer.pubspec, isNotNull);
expect(analyzer.pubspec.name, 'async');
expect(analyzer.pubspec.version, '1.13.3');
});

// I expect this test to fail eventually, because the resolved versions
// will differ.
// TODO: fix test to be robust on the resolved versions
test('pubspec.lock', () async {
expect(analyzer.pubspecLock, isNotNull);
expect(analyzer.pubspecLock.packages, hasLength(47));
expect(
analyzer.pubspecLock.packages[0],
new LockedDependency(
key: 'analyzer',
descriptionName: 'analyzer',
descriptionUrl: 'https://pub.dartlang.org',
source: 'hosted',
version: '0.30.0+2',
));
expect(
analyzer.pubspecLock.packages[17],
new LockedDependency(
key: 'http_parser',
descriptionName: 'http_parser',
descriptionUrl: 'https://pub.dartlang.org',
source: 'hosted',
version: '3.1.1',
));
});
});
});
}