Bug 1245916: XBL bindings should support global declarations in comments. r=miker

To properly lint XBL files we need to support things like import-globals-from
and other ESlint comment directives so we have to pass comments through to the
code blocks that ESlint parses. Unfortunately the way the XBL processor works
now is by passing a separate code block for every method/property/etc. in the
XBL and ESlint doesn't retain state across the blocks so we would have to prefix
every block with every comment. Instead this change makes us output just a
single block that roughly looks like this:

<comments>
var bindings = {
  "<binding-id>": {
    <binding-part-name>: function() { ... }
  }
}

This has some interesting bonuses. Defining the same ID twice will cause a lint
failure. Same for the same field in a binding. The line mapping is a little
harder and there are still a few lines that won't map directly back to the
original file but they should be rare cases. The only downside is that since
some bindings have the same binding declared differently for different platforms
we have to exclude those from linting for now.

MozReview-Commit-ID: CAsPt5dtf6T

--HG--
extra : rebase_source : 91a60ef0359ef53093fe197ed63dbc4e1a9f10a5
extra : source : 01675e4828b524c04a9057d68b41e9cc01ca1bb9
This commit is contained in:
Dave Townsend 2016-02-05 12:13:34 -08:00
Родитель 56d83fc4bd
Коммит 9d6c28bf7c
2 изменённых файлов: 135 добавлений и 53 удалений

Просмотреть файл

@ -189,6 +189,7 @@ toolkit/components/passwordmgr/**
# Uses preprocessing
toolkit/content/contentAreaUtils.js
toolkit/content/widgets/videocontrols.xml
toolkit/content/widgets/wizard.xml
toolkit/components/jsdownloads/src/DownloadIntegration.jsm
toolkit/components/search/nsSearchService.js
toolkit/components/url-classifier/**

Просмотреть файл

@ -36,12 +36,15 @@ function XMLParser(parser) {
parser.onopentag = this.onOpenTag.bind(this);
parser.onclosetag = this.onCloseTag.bind(this);
parser.ontext = this.onText.bind(this);
parser.onopencdata = this.onOpenCDATA.bind(this);
parser.oncdata = this.onCDATA.bind(this);
parser.oncomment = this.onComment.bind(this);
this.document = {
local: "#document",
uri: null,
children: [],
comments: [],
}
this._currentNode = this.document;
}
@ -56,8 +59,11 @@ XMLParser.prototype = {
namespace: tag.uri,
attributes: {},
children: [],
comments: [],
textContent: "",
textStart: { line: this.parser.line, column: this.parser.column },
textLine: this.parser.line,
textColumn: this.parser.column,
textEndLine: this.parser.line
}
for (let attr of Object.keys(tag.attributes)) {
@ -71,6 +77,7 @@ XMLParser.prototype = {
},
onCloseTag: function(tagname) {
this._currentNode.textEndLine = this.parser.line;
this._currentNode = this._currentNode.parentNode;
},
@ -83,30 +90,61 @@ XMLParser.prototype = {
this.addText(text.replace(entityRegex, "null"));
},
onCDATA: function(text) {
onOpenCDATA: function() {
// Turn the CDATA opening tag into whitespace for indent alignment
this.addText(" ".repeat("<![CDATA[".length));
},
onCDATA: function(text) {
this.addText(text);
},
onComment: function(text) {
this._currentNode.comments.push(text);
}
}
// Strips the indentation from lines of text and adds a fixed two spaces indent
function buildCodeBlock(tag, prefix, suffix, indent) {
prefix = prefix === undefined ? "" : prefix;
suffix = suffix === undefined ? "\n}" : suffix;
indent = indent === undefined ? 2 : indent;
// -----------------------------------------------------------------------------
// Processor Definition
// -----------------------------------------------------------------------------
let text = tag.textContent;
let line = tag.textStart.line;
let column = tag.textStart.column;
const INDENT_LEVEL = 2;
let lines = text.split("\n");
function indent(count) {
return " ".repeat(count * INDENT_LEVEL);
}
// Stores any XML parse error
let xmlParseError = null;
// Stores the lines of JS code generated from the XBL
let scriptLines = [];
// Stores a map from the synthetic line number to the real line number
// and column offset.
let lineMap = [];
function addSyntheticLine(line, linePos) {
lineMap[scriptLines.length] = { line: linePos, offset: null };
scriptLines.push(line);
}
/**
* Adds generated lines from an XBL node to the script to be passed back to eslint.
*/
function addNodeLines(node, reindent) {
let lines = node.textContent.split("\n");
let startLine = node.textLine;
let startColumn = node.textColumn;
// The case where there are no whitespace lines before the first text is
// treated differently for indentation
let indentFirst = false;
// Strip off any preceeding whitespace only lines. These are often used to
// format the XML and CDATA blocks.
while (lines.length && lines[0].trim() == "") {
column = 0;
line++;
indentFirst = true;
startLine++;
lines.shift();
}
@ -116,9 +154,13 @@ function buildCodeBlock(tag, prefix, suffix, indent) {
lines.pop();
}
// Indent the first line with the starting position of the text block
if (lines.length && column) {
lines[0] = " ".repeat(column) + lines[0];
if (!indentFirst) {
let firstLine = lines.shift();
firstLine = " ".repeat(reindent * INDENT_LEVEL) + firstLine;
// ESLint counts columns starting at 1 rather than 0
lineMap[scriptLines.length] = { line: startLine, offset: reindent * INDENT_LEVEL - (startColumn - 1) };
scriptLines.push(firstLine);
startLine++;
}
// Find the preceeding whitespace for all lines that aren't entirely whitespace
@ -127,33 +169,26 @@ function buildCodeBlock(tag, prefix, suffix, indent) {
// Find the smallest indent level in use
let minIndent = Math.min.apply(null, indents);
// Strip off the found indent level and prepend the new indent level, but only
// if the string isn't already empty.
let indentstr = " ".repeat(indent);
lines = lines.map(s => s.length ? indentstr + s.substring(minIndent) : s);
for (let line of lines) {
if (line.trim().length == 0) {
// Don't offset lines that are only whitespace, the only possible JS error
// is trailing whitespace and we want it to point at the right place
lineMap[scriptLines.length] = { line: startLine, offset: 0 };
} else {
line = " ".repeat(reindent * INDENT_LEVEL) + line.substring(minIndent);
lineMap[scriptLines.length] = { line: startLine, offset: reindent * INDENT_LEVEL - (minIndent - 1) };
}
let block = {
script: prefix + lines.join("\n") + suffix + "\n",
line: line - prefix.split("\n").length,
indent: minIndent - indent
};
return block;
scriptLines.push(line);
startLine++;
}
}
// -----------------------------------------------------------------------------
// Processor Definition
// -----------------------------------------------------------------------------
// Stores any XML parse error
let xmlParseError = null;
// Stores the code blocks
let blocks = [];
module.exports = {
preprocess: function(text, filename) {
xmlParseError = null;
blocks = [];
scriptLines = [];
lineMap = [];
// Non-strict allows us to ignore many errors from entities and
// preprocessing at the expense of failing to report some XML errors.
@ -181,19 +216,33 @@ module.exports = {
return [];
}
let scripts = [];
for (let comment of document.comments) {
addSyntheticLine(`/*`, 0);
for (let line of comment.split("\n")) {
addSyntheticLine(`${line.trim()}`, 0);
}
addSyntheticLine(`*/`, 0);
}
addSyntheticLine(`var bindings = {`, bindings.textLine);
for (let binding of bindings.children) {
if (binding.local != "binding" || binding.namespace != NS_XBL) {
continue;
}
addSyntheticLine(indent(1) + `"${binding.attributes.id}": {`, binding.textLine);
for (let part of binding.children) {
if (part.namespace != NS_XBL) {
continue;
}
if (part.local != "implementation" && part.local != "handlers") {
if (part.local == "implementation") {
addSyntheticLine(indent(2) + `implementation: {`, part.textLine);
} else if (part.local == "handlers") {
addSyntheticLine(indent(2) + `handlers: [`, part.textLine);
} else {
continue;
}
@ -204,28 +253,40 @@ module.exports = {
switch (item.local) {
case "field": {
// Fields get converted into variable declarations
// Fields are something like lazy getter functions
// Ignore empty fields
if (item.textContent.trim().length == 0) {
continue;
}
blocks.push(buildCodeBlock(item, "", "", 0));
addSyntheticLine(indent(3) + `get ${item.attributes.name}() {`, item.textLine);
// TODO This will probably break some style rules when. We need
// to inject this next to the first non-whitespace character
addSyntheticLine(indent(4) + `return`, item.textLine);
addNodeLines(item, 4);
addSyntheticLine(indent(3) + `},`, item.textEndLine);
break;
}
case "constructor":
case "destructor": {
// Constructors and destructors become function declarations
blocks.push(buildCodeBlock(item, `function ${item.local}() {\n`));
addSyntheticLine(indent(3) + `${item.local}() {`, item.textLine);
addNodeLines(item, 4);
addSyntheticLine(indent(3) + `},`, item.textEndLine);
break;
}
case "method": {
// Methods become function declarations with the appropriate params
let params = item.children.filter(n => n.local == "parameter" && n.namespace == NS_XBL)
.map(n => n.attributes.name)
.join(", ");
let body = item.children.filter(n => n.local == "body" && n.namespace == NS_XBL)[0];
blocks.push(buildCodeBlock(body, `function ${item.attributes.name}(${params}) {\n`));
addSyntheticLine(indent(3) + `${item.attributes.name}(${params}) {`, item.textLine);
addNodeLines(body, 4);
addSyntheticLine(indent(3) + `},`, item.textEndLine);
break;
}
case "property": {
@ -235,24 +296,38 @@ module.exports = {
continue;
}
let params = propdef.local == "setter" ? "val" : "";
blocks.push(buildCodeBlock(propdef, `function ${item.attributes.name}_${propdef.local}(${params}) {\n`));
if (propdef.local == "setter") {
addSyntheticLine(indent(3) + `set ${item.attributes.name}(val) {`, propdef.textLine);
} else if (propdef.local == "getter") {
addSyntheticLine(indent(3) + `get ${item.attributes.name}() {`, propdef.textLine);
} else {
continue;
}
addNodeLines(propdef, 4);
addSyntheticLine(indent(3) + `},`, propdef.textEndLine);
}
break;
}
case "handler": {
// Handlers become a function declaration with an `event` parameter
blocks.push(buildCodeBlock(item, `function onevent(event) {\n`));
addSyntheticLine(indent(3) + `function(event) {`, item.textLine);
addNodeLines(item, 4);
addSyntheticLine(indent(3) + `},`, item.textEndLine);
break;
}
default:
continue;
}
}
}
}
return blocks.map(b => b.script);
addSyntheticLine(indent(2) + (part.local == "implementation" ? `},` : `],`), part.textEndLine);
}
addSyntheticLine(indent(1) + `},`, binding.textEndLine);
}
addSyntheticLine(`};`, bindings.textEndLine);
let script = scriptLines.join("\n") + "\n";
return [script];
},
postprocess: function(messages, filename) {
@ -265,11 +340,17 @@ module.exports = {
// correct place.
let errors = [];
for (let i = 0; i < messages.length; i++) {
let block = blocks[i];
for (let message of messages[i]) {
message.line += block.line + 1;
message.column += block.indent;
// ESLint indexes lines starting at 1 but our arrays start at 0
let mapped = lineMap[message.line - 1];
message.line = mapped.line + 1;
if (mapped.offset) {
message.column -= mapped.offset;
} else {
message.column = NaN;
}
errors.push(message);
}
}