Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(273)

Unified Diff: pkg/polymer/lib/src/build/linter.dart

Issue 513023002: Step one towards stable error messages with details: (Closed) Base URL: https://dart.googlecode.com/svn/branches/bleeding_edge/dart
Patch Set: Created 6 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: pkg/polymer/lib/src/build/linter.dart
diff --git a/pkg/polymer/lib/src/build/linter.dart b/pkg/polymer/lib/src/build/linter.dart
index 3ec663769151c6e0897c15c8178257374c4d84b3..1fccef64ac801405e14b6c8bc79f3661cc686e71 100644
--- a/pkg/polymer/lib/src/build/linter.dart
+++ b/pkg/polymer/lib/src/build/linter.dart
@@ -11,13 +11,15 @@ import 'dart:convert';
import 'package:barback/barback.dart';
import 'package:code_transformers/assets.dart';
+import 'package:code_transformers/messages/build_logger.dart';
+import 'package:code_transformers/messages/messages.dart' show Message;
import 'package:html5lib/dom.dart';
import 'package:html5lib/dom_parsing.dart';
import 'package:source_span/source_span.dart';
import 'common.dart';
import 'utils.dart';
-import 'wrapped_logger.dart';
+import 'messages.dart';
/// A linter that checks for common Polymer errors and produces warnings to
/// show on the editor or the command line. Leaves sources unchanged, but
@@ -38,17 +40,17 @@ class Linter extends Transformer with PolymerTransformer {
seen.add(id);
bool isEntryPoint = options.isHtmlEntryPoint(id);
- var logger = options.releaseMode ? transform.logger :
- new WrappedLogger(transform, convertErrorsToWarnings: true);
+ var logger = new BuildLogger(transform,
+ convertErrorsToWarnings: !options.releaseMode);
- return readPrimaryAsHtml(transform).then((document) {
+ return readPrimaryAsHtml(transform, logger).then((document) {
return _collectElements(document, id, transform, logger, seen)
.then((elements) {
new _LinterVisitor(id, logger, elements, isEntryPoint).run(document);
- // Write out the logs collected by our [WrappedLogger].
- if (options.injectBuildLogsInOutput && logger is WrappedLogger) {
- return (logger as WrappedLogger).writeOutput();
+ // Write out the logs collected by our [BuildLogger].
+ if (options.injectBuildLogsInOutput && logger is BuildLogger) {
+ return (logger as BuildLogger).writeOutput();
}
});
});
@@ -60,7 +62,7 @@ class Linter extends Transformer with PolymerTransformer {
/// first.
Future<Map<String, _ElementSummary>> _collectElements(
Document document, AssetId sourceId, Transform transform,
- TransformLogger logger, Set<AssetId> seen,
+ BuildLogger logger, Set<AssetId> seen,
[Map<String, _ElementSummary> elements]) {
if (elements == null) elements = <String, _ElementSummary>{};
return _getImportedIds(document, sourceId, transform, logger)
@@ -81,17 +83,17 @@ class Linter extends Transformer with PolymerTransformer {
}
Future _readAndCollectElements(AssetId id, Transform transform,
- TransformLogger logger, Set<AssetId> seen,
+ BuildLogger logger, Set<AssetId> seen,
Map<String, _ElementSummary> elements) {
if (id == null || seen.contains(id)) return new Future.value(null);
seen.add(id);
- return readAsHtml(id, transform, showWarnings: false).then(
+ return readAsHtml(id, transform, logger, showWarnings: false).then(
(doc) => _collectElements(doc, id, transform, logger, seen, elements));
}
Future<List<AssetId>> _getImportedIds(
Document document, AssetId sourceId, Transform transform,
- TransformLogger logger) {
+ BuildLogger logger) {
var importIds = [];
for (var tag in document.querySelectorAll('link')) {
if (tag.attributes['rel'] != 'import') continue;
@@ -102,15 +104,15 @@ class Linter extends Transformer with PolymerTransformer {
importIds.add(assetExists(id, transform).then((exists) {
if (exists) return id;
if (sourceId == transform.primaryInput.id) {
- logger.warning('couldn\'t find imported asset "${id.path}" in package'
- ' "${id.package}".', span: span);
+ logger.warning(importNotFound.create(
+ {'path': id.path, 'package': id.package}), span: span);
}
}));
}
return Future.wait(importIds);
}
- void _addElements(Document document, TransformLogger logger,
+ void _addElements(Document document, BuildLogger logger,
Map<String, _ElementSummary> elements) {
for (var tag in document.querySelectorAll('polymer-element')) {
var name = tag.attributes['name'];
@@ -123,10 +125,12 @@ class Linter extends Transformer with PolymerTransformer {
// Report warning only once.
if (existing.hasConflict) continue;
existing.hasConflict = true;
- logger.warning('duplicate definition for custom tag "$name".',
+ logger.warning(duplicateDefinition.create(
+ {'name': name, 'second': ''}),
span: existing.span);
- logger.warning('duplicate definition for custom tag "$name" '
- ' (second definition).', span: span);
+ logger.warning(duplicateDefinition.create(
+ {'name': name, 'second': ' (second definition).'}),
+ span: span);
continue;
}
@@ -161,7 +165,7 @@ class _ElementSummary {
}
class _LinterVisitor extends TreeVisitor {
- TransformLogger _logger;
+ BuildLogger _logger;
AssetId _sourceId;
bool _inPolymerElement = false;
bool _dartTagSeen = false;
@@ -198,7 +202,7 @@ class _LinterVisitor extends TreeVisitor {
visit(doc);
if (_isEntryPoint && !_dartTagSeen && !_polymerExperimentalHtmlSeen) {
- _logger.warning(USE_INIT_DART, span: doc.body.sourceSpan);
+ _logger.warning(useInitDart, span: doc.body.sourceSpan);
}
}
@@ -208,20 +212,19 @@ class _LinterVisitor extends TreeVisitor {
if (rel != 'import' && rel != 'stylesheet') return;
if (rel == 'import' && _dartTagSeen) {
- _logger.warning("Move HTML imports above your Dart script tag.",
- span: node.sourceSpan);
+ _logger.warning(moveHtmlImportsUp, span: node.sourceSpan);
}
var href = node.attributes['href'];
if (href == null || href == '') {
- _logger.warning('link rel="$rel" missing href.', span: node.sourceSpan);
+ _logger.warning(missingHref.create({'rel': rel}), span: node.sourceSpan);
return;
}
if (rel != 'import') return;
if (_inPolymerElement) {
- _logger.error(NO_IMPORT_WITHIN_ELEMENT, span: node.sourceSpan);
+ _logger.error(noImportWithinElement, span: node.sourceSpan);
return;
}
@@ -233,8 +236,7 @@ class _LinterVisitor extends TreeVisitor {
/// Produce warnings if using `<element>` instead of `<polymer-element>`.
void _validateElementElement(Element node) {
- _logger.warning('<element> elements are not supported, use'
- ' <polymer-element> instead', span: node.sourceSpan);
+ _logger.warning(elementWasDeprecatedEonsAgo, span: node.sourceSpan);
}
/// Produce warnings if using `<polymer-element>` in the wrong place or if the
@@ -246,7 +248,7 @@ class _LinterVisitor extends TreeVisitor {
}
if (_inPolymerElement) {
- _logger.error('Nested polymer element definitions are not allowed.',
+ _logger.error(nestedPolymerElementDefinitionsNotAllowed,
span: node.sourceSpan);
return;
}
@@ -255,18 +257,14 @@ class _LinterVisitor extends TreeVisitor {
var extendsTag = node.attributes['extends'];
if (tagName == null) {
- _logger.error('Missing tag name of the custom element. Please include an '
- 'attribute like \'name="your-tag-name"\'.',
- span: node.sourceSpan);
+ _logger.error(missingTagName, span: node.sourceSpan);
} else if (!isCustomTagName(tagName)) {
- _logger.error('Invalid name "$tagName". Custom element names must have '
- 'at least one dash and can\'t be any of the following names: '
- '${invalidTagNames.keys.join(", ")}.',
+ _logger.error(invalidTagName.create({'name': tagName}),
span: node.sourceSpan);
}
if (_elements[extendsTag] == null && isCustomTagName(extendsTag)) {
- _logger.warning('custom element with name "$extendsTag" not found.',
+ _logger.warning(customElementNotFound.create({'tag': extendsTag}),
span: node.sourceSpan);
}
@@ -294,9 +292,9 @@ class _LinterVisitor extends TreeVisitor {
var src = node.attributes['src'];
if (isDart) {
- if (_dartTagSeen) _logger.warning(ONLY_ONE_TAG, span: node.sourceSpan);
+ if (_dartTagSeen) _logger.warning(onlyOneTag, span: node.sourceSpan);
if (_isEntryPoint && _polymerExperimentalHtmlSeen) {
- _logger.warning(NO_DART_SCRIPT_AND_EXPERIMENTAL, span: node.sourceSpan);
+ _logger.warning(noDartScriptAndExperimental, span: node.sourceSpan);
}
_dartTagSeen = true;
}
@@ -305,26 +303,23 @@ class _LinterVisitor extends TreeVisitor {
if (src == null) {
if (isDart && isEmpty) {
- _logger.warning('script tag seems empty.', span: node.sourceSpan);
+ _logger.warning(scriptTagSeemsEmpty, span: node.sourceSpan);
}
return;
}
if (src.endsWith('.dart') && !isDart) {
- _logger.warning('Wrong script type, expected type="application/dart".',
- span: node.sourceSpan);
+ _logger.warning(wrongScriptTypeExpectedDart, span: node.sourceSpan);
return;
}
if (!src.endsWith('.dart') && isDart) {
- _logger.warning('"application/dart" scripts should use the .dart file '
- 'extension.', span: node.sourceSpan);
+ _logger.warning(wrongFileTypeExpectedDart, span: node.sourceSpan);
return;
}
if (!isEmpty) {
- _logger.warning('script tag has "src" attribute and also has script '
- 'text.', span: node.sourceSpan);
+ _logger.warning(foundBothScriptSrcAndBody, span: node.sourceSpan);
}
}
@@ -359,39 +354,27 @@ class _LinterVisitor extends TreeVisitor {
var info = _elements[customTagName];
if (info == null) {
- // TODO(jmesserly): this warning is wrong if someone is using raw custom
- // elements. Is there another way we can handle this warning that won't
- // generate false positives?
- _logger.warning('definition for Polymer element with tag name '
- '"$customTagName" not found.', span: node.sourceSpan);
+ _logger.warning(customElementNotFound.create({'tag': customTagName}),
+ span: node.sourceSpan);
return;
}
var baseTag = info.baseExtendsTag;
if (baseTag != null && !hasIsAttribute) {
- _logger.warning(
- 'custom element "$customTagName" extends from "$baseTag", but '
- 'this tag will not include the default properties of "$baseTag". '
- 'To fix this, either write this tag as <$baseTag '
- 'is="$customTagName"> or remove the "extends" attribute from '
- 'the custom element declaration.', span: node.sourceSpan);
+ _logger.warning(incorrectInstantiationMissingBaseTag.create(
+ {'tag': customTagName, 'base': baseTag}), span: node.sourceSpan);
return;
}
if (hasIsAttribute && baseTag == null) {
- _logger.warning(
- 'custom element "$customTagName" doesn\'t declare any type '
- 'extensions. To fix this, either rewrite this tag as '
- '<$customTagName> or add \'extends="$nodeTag"\' to '
- 'the custom element declaration.', span: node.sourceSpan);
+ _logger.warning(incorrectInstantiationBogusBaseTag.create(
+ {'tag': customTagName, 'base': nodeTag}), span: node.sourceSpan);
return;
}
if (hasIsAttribute && baseTag != nodeTag) {
- _logger.warning(
- 'custom element "$customTagName" extends from "$baseTag". '
- 'Did you mean to write <$baseTag is="$customTagName">?',
- span: node.sourceSpan);
+ _logger.warning(incorrectInstantiationWrongBaseTag.create(
+ {'tag': customTagName, 'base': baseTag}), span: node.sourceSpan);
}
}
@@ -399,9 +382,9 @@ class _LinterVisitor extends TreeVisitor {
bool _validateCustomAttributeName(String name, FileSpan span) {
if (name.contains('-')) {
var newName = toCamelCase(name);
- _logger.warning('PolymerElement no longer recognizes attribute names with '
- 'dashes such as "$name". Use "$newName" or "${newName.toLowerCase()}" '
- 'instead (both forms are equivalent in HTML).', span: span);
+ var alternative = '"$newName" or "${newName.toLowerCase()}"';
+ _logger.warning(noDashesInCustomAttributes.create(
+ {'name': name, 'alternative': alternative}), span: span);
return false;
}
return true;
@@ -412,8 +395,7 @@ class _LinterVisitor extends TreeVisitor {
if (!name.startsWith('on-')) return;
if (!_inPolymerElement) {
- _logger.warning('Inline event handlers are only supported inside '
- 'declarations of <polymer-element>.',
+ _logger.warning(eventHandlersOnlyWithinPolymerElements,
span: node.attributeSpans[name]);
return;
}
@@ -423,18 +405,14 @@ class _LinterVisitor extends TreeVisitor {
// non empty.
if (!value.startsWith("{{") || !value.endsWith("}}") || value.contains('(')
|| value.substring(2, value.length - 2).trim() == '') {
- _logger.warning('Invalid event handler body "$value". Declare a method '
- 'in your custom element "void handlerName(event, detail, target)" '
- 'and use the form $name="{{handlerName}}".',
+ _logger.warning(invalidEventHandlerBody.create(
+ {'value': value, 'name': name}),
span: node.attributeSpans[name]);
}
}
}
-const String ONLY_ONE_TAG =
- 'Only one "application/dart" script tag per document is allowed.';
-
-String usePolymerHtmlMessageFrom(AssetId id) {
+Message usePolymerHtmlMessageFrom(AssetId id) {
var segments = id.path.split('/');
var upDirCount = 0;
if (segments[0] == 'lib') {
@@ -444,30 +422,9 @@ String usePolymerHtmlMessageFrom(AssetId id) {
// web/a/foo.html => ../packages/
upDirCount = segments.length - 2;
}
- return usePolymerHtmlMessage(upDirCount);
-}
-
-String usePolymerHtmlMessage(int upDirCount) {
var reachOutPrefix = '../' * upDirCount;
- return 'Missing definition for <polymer-element>, please add the following '
- 'HTML import at the top of this file: <link rel="import" '
- 'href="${reachOutPrefix}packages/polymer/polymer.html">.';
+ return usePolymerHtmlMessage.create({'reachOutPrefix': reachOutPrefix});
}
-const String NO_IMPORT_WITHIN_ELEMENT = 'Polymer.dart\'s implementation of '
- 'HTML imports are not supported within polymer element definitions, yet. '
- 'Please move the import out of this <polymer-element>.';
-
-const String USE_INIT_DART =
- 'To run a polymer application, you need to call "initPolymer". You can '
- 'either include a generic script tag that does this for you:'
- '\'<script type="application/dart">export "package:polymer/init.dart";'
- '</script>\' or add your own script tag and call that function. '
- 'Make sure the script tag is placed after all HTML imports.';
-
-const String NO_DART_SCRIPT_AND_EXPERIMENTAL =
- 'The experimental bootstrap feature doesn\'t support script tags on '
- 'the main document (for now).';
-
const List<String> INTERNALLY_DEFINED_ELEMENTS =
const ['auto-binding-dart', 'polymer-element'];

Powered by Google App Engine
This is Rietveld 408576698