Pub lint steel thread.

Author: pq

lib/src/ast.dart

@@ -7,6 +7,12 @@ library dart_lint.src.ast;
 
 import 'package:analyzer/src/generated/ast.dart';
 import 'package:analyzer/src/generated/element.dart';
+import 'package:analyzer/src/generated/scanner.dart';
+
+final _identifier = new RegExp(r'^([_a-zA-Z]+([_a-zA-Z0-9])*)$');
+
+/// Returns `true` if the given [id] is a Dart keyword.
+bool isKeyWord(String id) => Keyword.keywords.keys.contains(id);
 
 /// Returns `true` if the given [ClassMember] is a method.
 bool isMethod(ClassMember m) => m is MethodDeclaration;
@@ -75,6 +81,10 @@ bool isSimpleSetter(MethodDeclaration setter) {
   return false;
 }
 
+/// Returns `true` if the given [id] is a valid Dart identifier.
+bool isValidDartIdentifier(String id) =>
+    !isKeyWord(id) && _identifier.hasMatch(id);
+
 bool _checkForSimpleGetter(MethodDeclaration getter, Expression expression) {
   if (expression is SimpleIdentifier) {
     var staticElement = expression.staticElement;

lib/src/io.dart

@@ -7,7 +7,13 @@ library io;
 import 'dart:io';
 
 import 'package:linter/src/linter.dart';
+import 'package:linter/src/util.dart';
+import 'package:path/path.dart' as p;
 
+bool isDartFile(FileSystemEntity entry) => isDartFileName(entry.path);
+
+bool isPubspecFile(FileSystemEntity entry) =>
+    isPubspecFileName(p.basename(entry.path));
 
 /// Runs the linter on [file], skipping links and files not ending in the
 /// '.dart' extension.
@@ -35,8 +41,8 @@ bool lintFile(FileSystemEntity file, {String dartSdkPath, String packageRoot}) {
   }
 
   try {
-   linter.lintFile(file);
-   return true;
+    linter.lintFile(file);
+    return true;
   } catch (err, stack) {
     stderr.writeln('''An error occurred while linting $path
   Please report it at: github.com/dart-lang/dart_lint/issues
@@ -45,5 +51,3 @@ $stack''');
   }
   return false;
 }
-
-bool isDartFile(FileSystemEntity entry) => entry.path.endsWith('.dart');

lib/src/linter.dart

@@ -13,6 +13,7 @@ import 'package:analyzer/src/generated/source.dart';
 import 'package:analyzer/src/services/lint.dart';
 import 'package:analyzer/src/string_source.dart';
 import 'package:linter/src/analysis.dart';
+import 'package:linter/src/io.dart';
 import 'package:linter/src/pub.dart';
 import 'package:linter/src/rules.dart';
 
@@ -69,12 +70,15 @@ abstract class DartLinter {
   Iterable<AnalysisErrorInfo> lintLibrarySource(
       {String libraryName, String libraryContents});
 
-  Iterable<AnalysisErrorInfo> lintPubSpecSource({String contents});
+  Iterable<AnalysisErrorInfo> lintPubspecSource({String contents});
 }
 
 class Group {
 
   /// Defined rule groups.
+  static const Group PUB = const Group._('Pub',
+      link: const Hyperlink('See the <strong>Pubspec Format</strong>',
+          'https://www.dartlang.org/tools/pub/pubspec.html'));
   static const Group STYLE_GUIDE = const Group._('Style Guide',
       link: const Hyperlink('See the <strong>Style Guide</strong>',
           'https://www.dartlang.org/articles/style-guide/'));
@@ -89,6 +93,8 @@ class Group {
       case 'Styleguide':
       case 'Style Guide':
         return STYLE_GUIDE;
+      case 'Pub':
+        return PUB;
       default:
         return new Group._(name,
             custom: true, description: description, link: link);
@@ -211,6 +217,11 @@ abstract class LintRule extends Linter implements Comparable<LintRule> {
   /// Lint name.
   final CamelCaseString name;
 
+  /// Until pubspec analysis is pushed into the analyzer proper, we need to
+  /// do some extra book-keeping to keep track of details that will help us
+  /// constitute AnalysisErrorInfos.
+  final List<AnalysisErrorInfo> _locationInfo = <AnalysisErrorInfo>[];
+
   LintRule({String name, this.group, this.kind, this.description, this.details,
       this.maturity: Maturity.STABLE})
       : name = new CamelCaseString(name);
@@ -227,16 +238,25 @@ abstract class LintRule extends Linter implements Comparable<LintRule> {
   /// Return a visitor to be passed to pubspecs to perform lint
   /// analysis.
   /// Lint errors are reported via this [Linter]'s error [reporter].
-  PubSpecVisitor getPubSpecVisitor() => null;
+  PubSpecVisitor getPubspecVisitor() => null;
+
+  @override
+  AstVisitor getVisitor() => null;
 
   void reportLint(AstNode node) {
     reporter.reportErrorForNode(
         new LintCode(name.value, description), node, []);
   }
 
   void reportPubLint(PSNode node) {
-    reporter.reportErrorForOffset(new LintCode(name.value, description),
-        node.span.start.offset, node.span.length);
+    // Cache error and location info for creating AnalysisErrorInfos
+    var error = new AnalysisError.con2(reporter.source, node.span.start.offset,
+        node.span.length, new LintCode(name.value, description));
+
+    _locationInfo.add(new AnalysisErrorInfoImpl([error], new _LineInfo(node)));
+
+    // Then do the reporting
+    reporter.reportError(error);
   }
 }
 
@@ -317,7 +337,9 @@ class SourceLinter implements DartLinter, AnalysisErrorListener {
 
   @override
   Iterable<AnalysisErrorInfo> lintFile(File sourceFile) =>
-      _registerAndRun(() => new AnalysisDriver.forFile(sourceFile, options));
+      isPubspecFile(sourceFile)
+          ? _lintPubspecFile(sourceFile)
+          : _lintDartFile(sourceFile);
 
   @override
   Iterable<AnalysisErrorInfo> lintLibrarySource(
@@ -326,31 +348,39 @@ class SourceLinter implements DartLinter, AnalysisErrorListener {
               new _StringSource(libraryContents, libraryName), options));
 
   @override
-  Iterable<AnalysisErrorInfo> lintPubSpecSource({String contents}) {
+  Iterable<AnalysisErrorInfo> lintPubspecSource({String contents}) {
+    var results = <AnalysisErrorInfo>[];
 
     //TODO: error handling
     var spec = new PubSpec.parse(contents);
 
     for (Linter lint in options.enabledLints) {
       if (lint is LintRule) {
         LintRule rule = lint;
-        var visitor = rule.getPubSpecVisitor();
+        var visitor = rule.getPubspecVisitor();
         if (visitor != null) {
           try {
             spec.accept(visitor);
           } on Exception catch (e) {
             reporter.exception(new LinterException(e.toString()));
           }
+          results.addAll(rule._locationInfo);
         }
       }
     }
-    //TODO: collect and return errors
-    return null;
+
+    return results;
   }
 
   @override
   onError(AnalysisError error) => errors.add(error);
 
+  Iterable<AnalysisErrorInfo> _lintDartFile(File sourceFile) =>
+      _registerAndRun(() => new AnalysisDriver.forFile(sourceFile, options));
+
+  Iterable<AnalysisErrorInfo> _lintPubspecFile(File sourceFile) =>
+      lintPubspecSource(contents: sourceFile.readAsStringSync());
+
   Iterable<AnalysisErrorInfo> _registerAndRun(_DriverFactory createDriver) {
     _registerLinters(options.enabledLints);
     return createDriver().getErrors();
@@ -360,6 +390,15 @@ class SourceLinter implements DartLinter, AnalysisErrorListener {
       new LinterOptions(() => ruleMap.values);
 }
 
+class _LineInfo implements LineInfo {
+  PSNode node;
+  _LineInfo(this.node);
+
+  @override
+  LineInfo_Location getLocation(int offset) =>
+      new LineInfo_Location(node.span.start.line + 1, node.span.start.column);
+}
+
 class _StringSource extends StringSource {
   _StringSource(String contents, String fullName) : super(contents, fullName);
 

lib/src/rules.dart

@@ -10,6 +10,7 @@ import 'package:linter/src/rules/empty_constructor_bodies.dart';
 import 'package:linter/src/rules/library_names.dart';
 import 'package:linter/src/rules/library_prefixes.dart';
 import 'package:linter/src/rules/one_member_abstracts.dart';
+import 'package:linter/src/rules/pub/pub_package_names.dart';
 import 'package:linter/src/rules/super_goes_last.dart';
 import 'package:linter/src/rules/type_init_formals.dart';
 import 'package:linter/src/rules/unnecessary_brace_in_string_interp.dart';
@@ -20,9 +21,10 @@ import 'package:linter/src/rules/unnecessary_getters_setters.dart';
 final Map<String, LintRule> ruleMap = {
   'camel_case_types': new CamelCaseTypes(),
   'empty_constructor_bodies': new EmptyConstructorBodies(),
-  'library_names' : new LibraryNames(),
-  'library_prefixes' : new LibraryPrefixes(),
+  'library_names': new LibraryNames(),
+  'library_prefixes': new LibraryPrefixes(),
   'one_member_abstracts': new OneMemberAbstracts(),
+  'pub_package_names': new PubPackageNames(),
   'super_goes_last': new SuperGoesLast(),
   'type_init_formals': new TypeInitFormals(),
   'unnecessary_brace_in_string_interp': new UnnecessaryBraceInStringInterp(),

lib/src/rules/pub/pub_package_names.dart

@@ -0,0 +1,50 @@
+// Copyright (c) 2015, 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 linter.rules.pub.package_names;
+
+import 'package:linter/src/ast.dart';
+import 'package:linter/src/linter.dart';
+import 'package:linter/src/pub.dart';
+import 'package:linter/src/util.dart';
+
+const desc = 'DO use lowercase_with_underscores for package names';
+
+const details = '''
+**DO** use `lowercase_with_underscores` for package names.
+
+From the [Pubspec format description]
+(https://www.dartlang.org/tools/pub/pubspec.html):
+
+Package names should be all lowercase, with underscores to separate words,
+`just_like_this`. Use only basic Latin letters and Arabic digits: [a-z0-9_].
+Also, make sure the name is a valid Dart identifier -- that it doesn't start
+with digits and isn't a reserved word.
+''';
+
+bool isValidPackageName(String id) =>
+    isLowerCaseUnderScore(id) && isValidDartIdentifier(id);
+
+class PubPackageNames extends LintRule {
+  PubPackageNames() : super(
+          name: 'PubPackageNames',
+          description: desc,
+          details: details,
+          group: Group.PUB,
+          kind: Kind.DO);
+
+  @override
+  PubSpecVisitor getPubspecVisitor() => new Visitor(this);
+}
+
+class Visitor extends PubSpecVisitor {
+  LintRule rule;
+  Visitor(this.rule);
+
+  visitPackageName(PSEntry name) {
+    if (!isValidPackageName(name.value.text)) {
+      rule.reportPubLint(name.value);
+    }
+  }
+}

lib/src/util.dart

@@ -9,7 +9,13 @@ final _lowerCaseUnderScore = new RegExp(r'^([a-z]+([_]?[a-z]+))+$');
 final _lowerCaseUnderScoreWithDots =
     new RegExp(r'^([a-z]+([_]?[a-z]+))+(.([a-z]+([_]?[a-z]+)))*$');
 
+final _pubspec = new RegExp(r'^[_]?pubspec.yaml$');
+
+bool isDartFileName(String fileName) => fileName.endsWith('.dart');
+
 bool isLowerCaseUnderScore(String id) => _lowerCaseUnderScore.hasMatch(id);
 
 bool isLowerCaseUnderScoreWithDots(String id) =>
     _lowerCaseUnderScoreWithDots.hasMatch(id);
+
+bool isPubspecFileName(String fileName) => _pubspec.hasMatch(fileName);

pubspec.yaml

@@ -11,4 +11,5 @@ dependencies:
 dev_dependencies:
   markdown: '>=0.7.1+2 <0.8.0'
   mock: '>=0.11.0+2 <0.12.0'
+  mockito: '>=0.8.2 <0.9.0'
   unittest: '>=0.11.0 <0.12.0'