Arlolra has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/326062 )
Change subject: WIP: [linter] Use ext api for linter
......................................................................
WIP: [linter] Use ext api for linter
Change-Id: Ib0ff28c14c953751f0c20985c790a2c7faa9dedb
---
M lib/config/MWParserEnvironment.js
M lib/config/ParsoidConfig.js
M lib/config/WikiConfig.js
M lib/config/extapi.js
M lib/ext/Cite/index.js
R lib/ext/Linter/index.js
M lib/index.js
M lib/logger/ParsoidLogger.js
D lib/logger/linter.js
M lib/wt2html/DOMPostProcessor.js
R tests/mocha/linter.js
11 files changed, 168 insertions(+), 188 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/services/parsoid
refs/changes/62/326062/1
diff --git a/lib/config/MWParserEnvironment.js
b/lib/config/MWParserEnvironment.js
index 6c90362..ed359e1 100644
--- a/lib/config/MWParserEnvironment.js
+++ b/lib/config/MWParserEnvironment.js
@@ -9,7 +9,6 @@
var Batcher = require('../mw/Batcher.js').Batcher;
var Util = require('../utils/Util.js').Util;
var ParserPipelineFactory =
require('../wt2html/parser.js').ParserPipelineFactory;
-var Linter = require('../logger/linter.js').Linter;
var ParsoidLogger = require('../logger/ParsoidLogger.js').ParsoidLogger;
/**
@@ -121,7 +120,6 @@
// Sets ids on nodes and stores data-* attributes in a JSON blob
this.pageBundle = false;
- this.linter = new Linter(this);
this.configureLogging();
this.initializeForPageName(options.pageName || this.defaultPageName);
@@ -259,12 +257,8 @@
"fatal", "error", "warn", "info",
];
- if (this.conf.parsoid.linting && !this.conf.parsoid.linterSendAPI) {
- defaultLogLevels.push("lint");
- }
-
// Configure backends
- logger.registerLoggingBackends(defaultLogLevels, this.conf.parsoid,
this.linter);
+ logger.registerLoggingBackends(defaultLogLevels, this.conf.parsoid);
};
// The default page name (true name, without wikitext url encoding)
diff --git a/lib/config/ParsoidConfig.js b/lib/config/ParsoidConfig.js
index ba87fcd..7569e96 100644
--- a/lib/config/ParsoidConfig.js
+++ b/lib/config/ParsoidConfig.js
@@ -222,16 +222,16 @@
ParsoidConfig.prototype.addHTMLTemplateParameters = false;
/**
- * @property {boolean|Array} linting Whether to enable linter Backend.
- * Or an array of enabled lint types
+ * @property {boolean|Array} linting
+ * Whether to enable linting. Or, an array of enabled lint types.
*/
ParsoidConfig.prototype.linting = false;
/**
* @property {boolean} linterSendAPI
* Whether to send lint errors to the MW API
- * Requires the MW Linter extension to be installed
- * and configured.
+ * Requires the MW Linter extension to be installed and configured.
+ * Otherwise, logs at level "info/lint" when linting is enabled.
*/
ParsoidConfig.prototype.linterSendAPI = false;
diff --git a/lib/config/WikiConfig.js b/lib/config/WikiConfig.js
index f266f53..c51b10d 100644
--- a/lib/config/WikiConfig.js
+++ b/lib/config/WikiConfig.js
@@ -524,10 +524,7 @@
this.extensionTags.set(tag.name.toLowerCase(), tag);
}, this);
if (ext.config.hasOwnProperty('domPostProcessor')) {
- this.extensionPostProcessors.push({
- tag: tags.map(function(t) { return t.name;
}).join('+'),
- domPP: ext.config.domPostProcessor,
- });
+
this.extensionPostProcessors.push(ext.config.domPostProcessor);
}
if (ext.config.hasOwnProperty('styles')) {
ext.config.styles.forEach(function(s) {
diff --git a/lib/config/extapi.js b/lib/config/extapi.js
index 193e30e..15c3aee 100644
--- a/lib/config/extapi.js
+++ b/lib/config/extapi.js
@@ -31,8 +31,10 @@
Util: require('../utils/Util.js').Util,
DOMUtils: require('../utils/DOMUtils.js').DOMUtils,
JSUtils: require('../utils/jsutils.js').JSUtils,
+ DOMTraverser:
require('../utils/DOMTraverser.js').DOMTraverser,
addMetaData:
require('../wt2html/DOMPostProcessor.js').DOMPostProcessor.addMetaData,
defines: require('../wt2html/parser.defines.js'),
+ ApiRequest: require('../mw/ApiRequest.js'),
};
},
};
diff --git a/lib/ext/Cite/index.js b/lib/ext/Cite/index.js
index f523d80..abd0968 100644
--- a/lib/ext/Cite/index.js
+++ b/lib/ext/Cite/index.js
@@ -692,7 +692,10 @@
this.ref = new Ref(this);
this.references = new References(this);
this.config = {
- domPostProcessor: this.domPostProcessor.bind(this),
+ domPostProcessor: {
+ name: 'cite',
+ proc: this.domPostProcessor.bind(this),
+ },
tags: [
{
name: 'ref',
diff --git a/lib/wt2html/pp/handlers/linter.js b/lib/ext/Linter/index.js
similarity index 61%
rename from lib/wt2html/pp/handlers/linter.js
rename to lib/ext/Linter/index.js
index e5cf66d..fb07a6c 100644
--- a/lib/wt2html/pp/handlers/linter.js
+++ b/lib/ext/Linter/index.js
@@ -1,3 +1,95 @@
+'use strict';
+
+var ParsoidExtApi =
module.parent.require('./extapi.js').versionCheck('^0.6.1');
+
+var DU = ParsoidExtApi.DOMUtils;
+var Util = ParsoidExtApi.Util;
+var DOMTraverser = ParsoidExtApi.DOMTraverser;
+var LintRequest = ParsoidExtApi.ApiRequest.LintRequest;
+
+var Linter = function() {
+ this.buffer = [];
+ this.config = {
+ domPostProcessor: {
+ name: 'linter',
+ proc: this.domPostProcessor.bind(this),
+ },
+ };
+};
+
+Linter.prototype.domPostProcessor = function(node, env, options, atTopLevel) {
+ if (env.conf.parsoid.linting && atTopLevel) {
+ var domVisitor = new DOMTraverser(env);
+ domVisitor.addHandler(null, this.logWikitextFixups.bind(this));
+ domVisitor.traverse(node, env, options, atTopLevel);
+ if (env.conf.parsoid.linterSendAPI) {
+ this.sendToApi(env);
+ }
+ }
+};
+
+Linter.prototype.lint = function(env, type, lintObj) {
+ console.assert(env.conf.parsoid.linting, 'How did we get here?');
+
+ if (!env.conf.parsoid.linterSendAPI) {
+ env.log('info/lint/' + type, lintObj);
+ return;
+ }
+
+ var msg = {
+ type: type,
+ params: lintObj.params || {},
+ };
+
+ var dsr = lintObj.dsr;
+ if (dsr) {
+ msg.dsr = dsr;
+ }
+
+ if (lintObj.templateInfo) {
+ msg.templateInfo = lintObj.templateInfo;
+ }
+
+ var src = lintObj.src;
+ if (type === 'fostered' || type === 'multi-template' || type ===
'mixed-content') {
+ msg.src = src;
+ } else if (dsr) {
+ msg.src = src.substring(dsr[0], dsr[1]);
+ }
+
+ this.buffer.push(msg);
+};
+
+Linter.prototype.sendToApi = function(env) {
+ console.assert(env.conf.parsoid.linterSendAPI, 'How did we get here?');
+
+ var enabledBuffer;
+ if (env.conf.parsoid.linting === true) {
+ enabledBuffer = this.buffer; // Everything is enabled
+ } else if (Array.isArray(env.conf.parsoid.linting)) {
+ enabledBuffer = this.buffer.filter(function(item) {
+ return env.conf.parsoid.linting.indexOf(item.type) !==
-1;
+ });
+ } else {
+ console.assert(false, 'Why are we here? Linting is disabled.');
+ }
+
+ if (env.page.id % env.conf.parsoid.linterAPISampling !== 0) {
+ return;
+ }
+
+ // Only send the request if it the latest revision
+ if (env.page.meta.revision.revid === env.page.latest) {
+ LintRequest.promise(env, JSON.stringify(enabledBuffer))
+ .then(function(data) {
+ if (data.error) { env.log('error/lint-api',
data.error); }
+ })
+ .catch(function(e) {
+ env.log('error/lint-api', e);
+ });
+ }
+};
+
/*
* DOM pass that walks the DOM tree and places a call to logger
* with logtype 'lint/*' to log the following scenarios:
@@ -10,10 +102,39 @@
* 6. Obsolete HTML tags
* 7. Self-closed HTML tags
*/
-'use strict';
+Linter.prototype.logWikitextFixups = function(node, env, atTopLevel, tplInfo) {
+ // For now, don't run linter in subpipelines.
+ // Only on the final DOM for the top-level page.
+ if (!atTopLevel || !DU.isElt(node)) {
+ return true;
+ }
-var DU = require('../../../utils/DOMUtils.js').DOMUtils;
-var Util = require('../../../utils/Util.js').Util;
+ var dp = DU.getDataParsoid(node);
+
+ if (tplInfo && tplInfo.first === node) {
+ // Log transclusions with more than one part
+ this.logTransclusions(env, node, dp, tplInfo);
+ }
+
+ // Log Tree Builder fixups
+ this.logTreeBuilderFixup(env, node, dp, tplInfo);
+
+ // Log Ignored Table Attributes
+ this.logIgnoredTableAttr(env, node, dp, tplInfo);
+
+ // Log obsolete HTML tags
+ this.logObsoleteHTMLTags(env, node, dp, tplInfo);
+
+ // Log bogus image options
+ this.logBogusImageOptions(env, node, dp, tplInfo);
+
+ if (dp.fostered) {
+ // Log Fostered content
+ return this.logfFosteredContent(env, node, dp, tplInfo);
+ } else {
+ return true;
+ }
+};
/*
* Log Transclusion with more than one parts
@@ -27,7 +148,7 @@
*
https://www.mediawiki.org/wiki/Parsoid/MediaWiki_DOM_spec#Transclusion_content
*/
-function logTransclusions(env, node, dp, tplInfo) {
+Linter.prototype.logTransclusions = function(env, node, dp, tplInfo) {
var dmw = DU.getDataMw(node);
if (dmw) {
var dsr = tplInfo.dsr;
@@ -36,7 +157,7 @@
var lintObj;
if (typeof parts[0] === 'string' || typeof
parts[parts.length - 1] === 'string') {
lintObj = {src: env.page.src, dsr: dsr };
- env.log('lint/mixed-content', lintObj);
+ this.lint(env, 'mixed-content', lintObj);
} else if (parts.length > 1) {
var targets = [];
dmw.parts.forEach(function(a) {
@@ -46,12 +167,12 @@
});
if (targets.length > 1) {
lintObj = { src: targets, dsr: dsr };
- env.log('lint/multi-template', lintObj);
+ this.lint(env, 'multi-template',
lintObj);
}
}
}
}
-}
+};
/*
* Log Treebuilder fixups marked by dom.markTreeBuilderFixup.js
@@ -61,7 +182,7 @@
* 2. Unclosed start tags
* 3. Stripped tags
*/
-function logTreeBuilderFixup(env, c, dp, tplInfo) {
+Linter.prototype.logTreeBuilderFixup = function(env, c, dp, tplInfo) {
var cNodeName = c.nodeName.toLowerCase();
var dsr = dp.dsr;
var lintObj;
@@ -78,7 +199,7 @@
var type = c.getAttribute('typeof');
if (type === 'mw:Placeholder/StrippedTag') {
lintObj = { src: env.page.src, dsr: dsr, templateInfo:
templateInfo };
- env.log('lint/stripped-tag', lintObj);
+ this.lint(env, 'stripped-tag', lintObj);
}
}
@@ -98,7 +219,7 @@
templateInfo: templateInfo,
params: { name: cNodeName },
};
- env.log('lint/self-closed-tag', lintObj);
+ this.lint(env, 'self-closed-tag', lintObj);
// The other checks won't pass - no need to test them.
return;
}
@@ -110,7 +231,7 @@
templateInfo: templateInfo,
params: { name: cNodeName },
};
- env.log('lint/missing-end-tag', lintObj);
+ this.lint(env, 'missing-end-tag', lintObj);
}
if (dp.autoInsertedStart === true && (tplInfo || dsr[3] > 0)) {
@@ -119,10 +240,10 @@
dsr: dsr,
templateInfo: templateInfo,
};
- env.log('lint/missing-start-tag', lintObj);
+ this.lint(env, 'missing-start-tag', lintObj);
}
}
-}
+};
/*
* Log ignored table attributes.
@@ -135,7 +256,7 @@
*
* Here foo gets ignored and is found in the data-parsoid of <tr> tags.
*/
-function logIgnoredTableAttr(env, c, dp, tplInfo) {
+Linter.prototype.logIgnoredTableAttr = function(env, c, dp, tplInfo) {
var dsr;
var templateInfo;
if (DU.hasNodeName(c, "table")) {
@@ -165,7 +286,7 @@
dsr =
dp.dsr;
}
var lintObj = {
src: env.page.src, dsr: dsr, templateInfo: templateInfo };
-
env.log('lint/ignored-table-attr', lintObj);
+ this.lint(env,
'ignored-table-attr', lintObj);
}
}
}
@@ -175,7 +296,7 @@
fc = fc.nextSibling;
}
}
-}
+};
/*
* Log fostered content marked by markFosteredContent.js
@@ -189,7 +310,7 @@
*
* Here 'foo' gets fostered out.
*/
-function logFosteredContent(env, node, dp, tplInfo) {
+Linter.prototype.logFosteredContent = function(env, node, dp, tplInfo) {
var fosteredSRC = node.innerHTML;
var nextSibling = node.nextSibling;
while (nextSibling && !DU.hasNodeName(nextSibling, 'table')) {
@@ -208,9 +329,9 @@
dsr = DU.getDataParsoid(nextSibling).dsr;
}
var lintObj = { src: fosteredSRC, dsr: dsr, templateInfo: templateInfo
};
- env.log('lint/fostered', lintObj);
+ this.lint(env, 'fostered', lintObj);
return nextSibling;
-}
+};
/*
*
@@ -218,7 +339,7 @@
* See - http://www.w3.org/TR/html5/obsolete.html#non-conforming-features
*
*/
-function logObsoleteHTMLTags(env, c, dp, tplInfo) {
+Linter.prototype.logObsoleteHTMLTags = function(env, c, dp, tplInfo) {
var re = /^(BIG|CENTER|FONT|STRIKE|TT)$/;
if (!(dp.autoInsertedStart && dp.autoInsertedEnd) &&
re.test(c.nodeName)) {
@@ -232,17 +353,17 @@
templateInfo: templateInfo,
params: { name: c.nodeName.toLowerCase() },
};
- env.log('lint/obsolete-tag', lintObj);
+ this.lint(env, 'obsolete-tag', lintObj);
}
-}
+};
/*
*
* Log bogus (=unrecognized) image options
-* See - https://www.mediawiki.org/wiki/Help:Images#Syntax
+* See - https://www.mediawiki.org/wiki/Help:Images#Syntax
*
*/
-function logBogusImageOptions(env, c, dp, tplInfo) {
+Linter.prototype.logBogusImageOptions = function(env, c, dp, tplInfo) {
if (DU.isGeneratedFigure(c)) {
var items = [];
dp.optList.forEach(function(item) {
@@ -255,7 +376,7 @@
if (tplInfo) {
templateInfo = { name:
DU.findEnclosingTemplateName(tplInfo) };
}
- env.log('lint/bogus-image-options', {
+ this.lint(env, 'bogus-image-options', {
src: env.page.src,
dsr: tplInfo ? tplInfo.dsr : dp.dsr,
templateInfo: templateInfo,
@@ -263,42 +384,8 @@
});
}
}
-}
+};
-function logWikitextFixups(node, env, atTopLevel, tplInfo) {
- // For now, don't run linter in subpipelines.
- // Only on the final DOM for the top-level page.
- if (!atTopLevel || !DU.isElt(node)) {
- return true;
- }
-
- var dp = DU.getDataParsoid(node);
-
- if (tplInfo && tplInfo.first === node) {
- // Log transclusions with more than one part
- logTransclusions(env, node, dp, tplInfo);
- }
-
- // Log Tree Builder fixups
- logTreeBuilderFixup(env, node, dp, tplInfo);
-
- // Log Ignored Table Attributes
- logIgnoredTableAttr(env, node, dp, tplInfo);
-
- // Log obsolete HTML tags
- logObsoleteHTMLTags(env, node, dp, tplInfo);
-
- // Log bogus image options
- logBogusImageOptions(env, node, dp, tplInfo);
-
- if (dp.fostered) {
- // Log Fostered content
- return logFosteredContent(env, node, dp, tplInfo);
- } else {
- return true;
- }
-}
-
-if (typeof module === "object") {
- module.exports.logWikitextFixups = logWikitextFixups;
+if (typeof module === 'object') {
+ module.exports = Linter;
}
diff --git a/lib/index.js b/lib/index.js
index 5238bb0..80c2dd9 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -174,8 +174,7 @@
// By default, set the loggerBackend and metrics to service-runner's.
var parsoidOptions = {
loggerBackend: function(logData, cb) {
- var type = logData.logType.replace(/^lint/,
'info/lint');
- options.logger.log(type, prepareLog(logData));
+ options.logger.log(logData.logType,
prepareLog(logData));
cb();
},
metrics: options.metrics,
diff --git a/lib/logger/ParsoidLogger.js b/lib/logger/ParsoidLogger.js
index 15973b2..54b358c 100644
--- a/lib/logger/ParsoidLogger.js
+++ b/lib/logger/ParsoidLogger.js
@@ -58,7 +58,7 @@
return this._defaultTracerBackend.bind(this);
};
-ParsoidLogger.prototype.registerLoggingBackends = function(defaultLogLevels,
parsoidConfig, linter) {
+ParsoidLogger.prototype.registerLoggingBackends = function(defaultLogLevels,
parsoidConfig) {
// Register a default backend based on default logTypes.
// DEFAULT: Combine all regexp-escaped default logTypes into a single
regexp.
var fixLogType = function(logType) { return Util.escapeRegExp(logType)
+ "(\\/|$)"; };
@@ -112,10 +112,6 @@
} else if (parsoidConfig.debugFlags) {
this.registerBackend(buildTraceOrDebugFlag(parsoidConfig.debugFlags, "debug"),
tracerBackend);
- }
- if (linter && parsoidConfig.linting) {
- this.registerBackend(/^lint(\/.*)?/,
linter.linterBackend.bind(linter));
- this.registerBackend(/^end(\/.*)/,
linter.logLintOutput.bind(linter));
}
};
diff --git a/lib/logger/linter.js b/lib/logger/linter.js
deleted file mode 100644
index dd8d74b..0000000
--- a/lib/logger/linter.js
+++ /dev/null
@@ -1,92 +0,0 @@
-/*
- * Logger backend for linter.
- * This backend filters out logging messages with Logtype "lint/*" and
- * logs them (console, external service).
- */
-
-'use strict';
-
-var LintRequest = require('../mw/ApiRequest.js').LintRequest;
-
-var Linter = function(env) {
- this._env = env;
- this.buffer = [];
-};
-
-Linter.prototype.logLintOutput = function(logData, cb) {
- var env = this._env;
- var enabledBuffer;
- try {
- // Everything is enabled
- if (env.conf.parsoid.linting === true) {
- enabledBuffer = this.buffer;
- } else if (Array.isArray(env.conf.parsoid.linting)) {
- enabledBuffer = this.buffer.filter(function(item) {
- return
env.conf.parsoid.linting.indexOf(item.type) !== -1;
- });
- }
-
- this.buffer = [];
-
- if (env.page.id % env.conf.parsoid.linterAPISampling !== 0) {
- return;
- }
-
- if (env.conf.parsoid.linterSendAPI &&
- // Only send the request if it the latest
revision
- env.page.meta.revision.revid ===
env.page.latest) {
- LintRequest.promise(env, JSON.stringify(enabledBuffer))
- .then(function(data) {
- if (data.error) { env.log('error/lint-api',
data.error); }
- })
- .catch(function(e) {
- env.log('error/lint-api', e);
- });
- }
- } catch (e) {
- env.log('error/lint-api', "Error in logLintOutput: " + e);
- } finally {
- cb();
- }
-};
-
-Linter.prototype.linterBackend = function(logData, cb) {
- // Wrap in try-catch-finally so we can more accurately
- // pin errors to specific logging backends
- try {
- var lintObj = logData.logObject[0];
-
- var msg = {
- type: logData.logType.match(/lint\/(.*)/)[1],
- params: lintObj.params || {},
- };
-
- var dsr = lintObj.dsr;
- if (dsr) {
- msg.dsr = dsr;
- }
-
- if (lintObj.templateInfo) {
- msg.templateInfo = lintObj.templateInfo;
- }
-
- var src = lintObj.src;
- if (msg.type === 'fostered' ||
- msg.type === 'multi-template' ||
- msg.type === 'mixed-content') {
- msg.src = src;
- } else if (dsr) {
- msg.src = src.substring(dsr[0], dsr[1]);
- }
-
- this.buffer.push(msg);
- } catch (e) {
- this._env.log("error/linter", "Error in linterBackend: " + e);
- } finally {
- cb();
- }
-};
-
-if (typeof module === "object") {
- module.exports.Linter = Linter;
-}
diff --git a/lib/wt2html/DOMPostProcessor.js b/lib/wt2html/DOMPostProcessor.js
index 308cdb1..93648cd 100644
--- a/lib/wt2html/DOMPostProcessor.js
+++ b/lib/wt2html/DOMPostProcessor.js
@@ -24,7 +24,6 @@
var wrapTemplates = require('./pp/processors/wrapTemplates.js').wrapTemplates;
// handlers
-var logWikitextFixup = require('./pp/handlers/linter.js').logWikitextFixups;
var CleanUp = require('./pp/handlers/cleanup.js');
var headings = require('./pp/handlers/headings.js');
var unpackDOMFragments =
require('./pp/handlers/unpackDOMFragments.js').unpackDOMFragments;
@@ -137,19 +136,13 @@
// A pure DOM transformation
env.conf.wiki.extensionPostProcessors.forEach(function(extPP) {
- addPP('tag:' + extPP.tag, extPP.domPP);
- }, this);
+ addPP(extPP.name, extPP.proc);
+ });
// Strip empty elements from template content
domVisitor = new DOMTraverser(env);
domVisitor.addHandler(null, CleanUp.stripEmptyElements);
addPP('stripEmptyElts', domVisitor.traverse.bind(domVisitor));
-
- if (env.conf.parsoid.linting) {
- domVisitor = new DOMTraverser(env);
- domVisitor.addHandler(null, logWikitextFixup);
- addPP('linter', domVisitor.traverse.bind(domVisitor));
- }
domVisitor = new DOMTraverser(env);
var tableFixer = new TableFixups(env);
diff --git a/tests/mocha/lintertest.js b/tests/mocha/linter.js
similarity index 98%
rename from tests/mocha/lintertest.js
rename to tests/mocha/linter.js
index 05871ff..dc94743 100644
--- a/tests/mocha/lintertest.js
+++ b/tests/mocha/linter.js
@@ -1,9 +1,10 @@
/** Test cases for the linter */
'use strict';
-require('../../core-upgrade.js');
/*global describe, it*/
-var should = require("chai").should(); /*jshint unused:false*/
+require('../../core-upgrade.js');
+require('chai').should();
+
var ParsoidConfig = require('../../lib/config/ParsoidConfig.js').ParsoidConfig;
var helpers = require('./test.helpers.js');
--
To view, visit https://gerrit.wikimedia.org/r/326062
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib0ff28c14c953751f0c20985c790a2c7faa9dedb
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/services/parsoid
Gerrit-Branch: master
Gerrit-Owner: Arlolra <abrea...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
