jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/384198 )
Change subject: Linter: Provide accurate DSR offsets for issues in extension
content
......................................................................
Linter: Provide accurate DSR offsets for issues in extension content
* DSR info was being blindly updated to entire extension range
instead of looking inside the extension content.
* If looking inside the extension content doesn't yield a valid dsr,
it falls back to the currently existing mode of returning DSR for
the extension use.
* Simplified some code structure as well.
* Updated mocha test that now has updated DSR offsets.
Bug: T178217
Change-Id: I7d99c1771fb4d55866d296ecd283202bfc62a8ed
---
M lib/wt2html/pp/handlers/linter.js
M tests/mocha/linter.js
2 files changed, 66 insertions(+), 97 deletions(-)
Approvals:
jenkins-bot: Verified
Arlolra: Looks good to me, approved
diff --git a/lib/wt2html/pp/handlers/linter.js
b/lib/wt2html/pp/handlers/linter.js
index fbd4d24..87d1ba0 100644
--- a/lib/wt2html/pp/handlers/linter.js
+++ b/lib/wt2html/pp/handlers/linter.js
@@ -145,6 +145,10 @@
* @return {string}
*/
function findEnclosingTemplateName(env, tplInfo) {
+ if (!tplInfo) {
+ return undefined;
+ }
+
var typeOf = tplInfo.first.getAttribute('typeof');
if (!/(?:^|\s)mw:Transclusion(?=$|\s)/.test(typeOf)) {
return undefined;
@@ -161,6 +165,14 @@
return { name: name };
} else {
return { multiPartTemplateBlock: true };
+ }
+}
+
+function findLintDSR(tplLintInfo, tplInfo, nodeDSR, updateNodeDSR) {
+ if (tplLintInfo || (tplInfo && !Util.isValidDSR(nodeDSR))) {
+ return tplInfo.dsr;
+ } else {
+ return updateNodeDSR ? updateNodeDSR(nodeDSR) : nodeDSR;
}
}
@@ -223,22 +235,13 @@
return;
}
- var cNodeName = c.nodeName.toLowerCase();
- var dsr = dp.dsr;
+ var templateInfo = findEnclosingTemplateName(env, tplInfo);
+ // During DSR computation, stripped meta tags
+ // surrender their width to its previous sibling.
+ // We record the original DSR in the tmp attribute
+ // for that reason.
+ var dsr = findLintDSR(templateInfo, tplInfo, dp.tmp.origDSR || dp.dsr);
var lintObj;
- var templateInfo;
-
- if (tplInfo) {
- dsr = tplInfo.dsr;
- templateInfo = findEnclosingTemplateName(env, tplInfo);
- } else if (dp.tmp.origDSR) {
- // During DSR computation, stripped meta tags
- // surrender their width to its previous sibling.
- // We record the original DSR in the tmp attribute
- // for that reason.
- dsr = dp.tmp.origDSR;
- }
-
if (DU.isMarkerMeta(c, 'mw:Placeholder/StrippedTag')) {
lintObj = { dsr: dsr, templateInfo: templateInfo, params: {
name: dp.name } };
env.log('lint/stripped-tag', lintObj);
@@ -254,6 +257,7 @@
// 3. c is not a HTML element (unless they are i/b quotes)
//
// 4. c doesn't have DSR info and doesn't come from a template either
+ var cNodeName = c.nodeName.toLowerCase();
if (!Util.isVoidElement(cNodeName) &&
cNodeName !== 'tbody' &&
(DU.hasLiteralHTMLMarker(dp) || DU.isQuoteElt(c)) &&
@@ -347,8 +351,6 @@
* Comment out this whole thing to satisfy eslint
*
function logIgnoredTableAttr(env, node, dp, tplInfo) {
- var dsr;
- var templateInfo;
if (!node.nodeName === 'TABLE') {
return;
}
@@ -365,13 +367,8 @@
if (!re.test(key) ||
!re.test(dp.sa[key])) {
// FIXME: This is incorrect.
// We need to check if the key
is absent in dp.a / in the node's attribute
- if (tplInfo) {
- dsr = tplInfo.dsr;
- templateInfo =
findEnclosingTemplateName(env, tplInfo);
- } else {
- dsr = dp.dsr;
- }
- var lintObj = { dsr: dsr,
templateInfo: templateInfo };
+ var templateInfo =
findEnclosingTemplateName(env, tplInfo);
+ var lintObj = { dsr:
findLintDSR(templateInfo, tplInfo, dp.dsr), templateInfo: templateInfo };
env.log('lint/ignored-table-attr', lintObj);
}
}
@@ -420,15 +417,11 @@
// Let us carry on with regularly scheduled programming.
}
- var dsr;
- var templateInfo;
- if (tplInfo) {
- dsr = tplInfo.dsr;
- templateInfo = findEnclosingTemplateName(env, tplInfo);
- } else {
- dsr = DU.getDataParsoid(maybeTable).dsr;
- }
- var lintObj = { dsr: dsr, templateInfo: templateInfo };
+ var templateInfo = findEnclosingTemplateName(env, tplInfo);
+ var lintObj = {
+ dsr: findLintDSR(templateInfo, tplInfo,
DU.getDataParsoid(maybeTable).dsr),
+ templateInfo: templateInfo
+ };
env.log('lint/fostered', lintObj);
return maybeTable;
@@ -451,12 +444,9 @@
}
if (!(dp.autoInsertedStart && dp.autoInsertedEnd) &&
obsoleteTagsRE.test(c.nodeName)) {
- var templateInfo;
- if (tplInfo) {
- templateInfo = findEnclosingTemplateName(env, tplInfo);
- }
+ var templateInfo = findEnclosingTemplateName(env, tplInfo);
var lintObj = {
- dsr: tplInfo ? tplInfo.dsr : dp.dsr,
+ dsr: findLintDSR(templateInfo, tplInfo, dp.dsr),
templateInfo: templateInfo,
params: { name: c.nodeName.toLowerCase() },
};
@@ -477,12 +467,9 @@
}
});
if (items.length) {
- var templateInfo;
- if (tplInfo) {
- templateInfo = findEnclosingTemplateName(env,
tplInfo);
- }
+ var templateInfo = findEnclosingTemplateName(env,
tplInfo);
env.log('lint/bogus-image-options', {
- dsr: tplInfo ? tplInfo.dsr : dp.dsr,
+ dsr: findLintDSR(templateInfo, tplInfo, dp.dsr),
templateInfo: templateInfo,
params: { items: items },
});
@@ -509,24 +496,23 @@
* |}
*/
function logDeletableTables(env, c, dp, tplInfo) {
- var templateInfo;
if (c.nodeName === 'TABLE') {
var prev = DU.previousNonSepSibling(c);
if (prev && prev.nodeName === 'TABLE' &&
DU.getDataParsoid(prev).autoInsertedEnd) {
- var dsr;
- if (tplInfo) {
- templateInfo = findEnclosingTemplateName(env,
tplInfo);
- dsr = tplInfo.dsr;
- } else {
- // Identify the dsr-span of the opening tag
- // of the table that needs to be deleted
- dsr = Util.clone(dp.dsr);
- if (dsr[2]) {
- dsr[1] = dsr[0] + dsr[2];
- dsr[2] = 0;
- dsr[3] = 0;
+ var templateInfo = findEnclosingTemplateName(env,
tplInfo);
+ var dsr = findLintDSR(templateInfo, tplInfo, dp.dsr,
+ function(nodeDSR) {
+ // Identify the dsr-span of the opening
tag
+ // of the table that needs to be deleted
+ var x = Util.clone(nodeDSR);
+ if (x[2]) {
+ x[1] = x[0] + x[2];
+ x[2] = 0;
+ x[3] = 0;
+ }
+ return x;
}
- }
+ );
var lintObj = {
dsr: dsr,
templateInfo: templateInfo,
@@ -561,25 +547,17 @@
}
function logBadPWrapping(env, node, dp, tplInfo) {
- var findP = function(e) { return e.nodeName === 'P'; };
- if (!DU.isBlockNode(node) && DU.isBlockNode(node.parentNode)) {
- if (hasNoWrapCSS(node)) {
- var p = findMatchingChild(node, findP);
- if (p) {
- var dsr, templateInfo;
- if (tplInfo) {
- templateInfo =
findEnclosingTemplateName(env, tplInfo);
- dsr = tplInfo.dsr;
- } else {
- dsr = dp.dsr;
- }
- var lintObj = {
- dsr: dsr,
- templateInfo: templateInfo,
- params: { root:
node.parentNode.nodeName, child: node.nodeName },
- };
- env.log('lint/pwrap-bug-workaround', lintObj);
- }
+ if (!DU.isBlockNode(node) && DU.isBlockNode(node.parentNode) &&
hasNoWrapCSS(node)) {
+ var findP = function(e) { return e.nodeName === 'P'; };
+ var p = findMatchingChild(node, findP);
+ if (p) {
+ var templateInfo = findEnclosingTemplateName(env,
tplInfo);
+ var lintObj = {
+ dsr: findLintDSR(templateInfo, tplInfo, dp.dsr),
+ templateInfo: templateInfo,
+ params: { root: node.parentNode.nodeName,
child: node.nodeName },
+ };
+ env.log('lint/pwrap-bug-workaround', lintObj);
}
}
}
@@ -698,29 +676,20 @@
// For every node where Tidy hoists whitespace,
// emit an event to flag a whitespace fixup opportunity.
- var dsr, templateInfo;
- if (tplInfo) {
- templateInfo = findEnclosingTemplateName(env, tplInfo);
- dsr = tplInfo.dsr;
- }
+ var templateInfo = findEnclosingTemplateName(env, tplInfo);
var n = nowrapNodes.length - 1;
nowrapNodes.forEach(function(o, i) {
- if (o.tidybug && i < n) {
- if (!tplInfo) {
- dsr = DU.getDataParsoid(o.node).dsr;
- }
- if (!nowrapNodes[i + 1].hasLeadingWS) {
- var lintObj = {
- dsr: dsr,
- templateInfo: templateInfo,
- params: {
- node: o.node.nodeName,
- sibling:
o.node.nextSibling.nodeName,
- },
- };
+ if (o.tidybug && i < n && !nowrapNodes[i + 1].hasLeadingWS) {
+ var lintObj = {
+ dsr: findLintDSR(templateInfo, tplInfo,
DU.getDataParsoid(o.node).dsr),
+ templateInfo: templateInfo,
+ params: {
+ node: o.node.nodeName,
+ sibling: o.node.nextSibling.nodeName,
+ },
+ };
- env.log('lint/tidy-whitespace-bug', lintObj);
- }
+ env.log('lint/tidy-whitespace-bug', lintObj);
}
});
diff --git a/tests/mocha/linter.js b/tests/mocha/linter.js
index 29d360f..33d68df 100644
--- a/tests/mocha/linter.js
+++ b/tests/mocha/linter.js
@@ -180,7 +180,7 @@
result.should.have.length(1);
result[0].should.have.a.property('type',
'obsolete-tag');
result[0].should.not.have.a.property('templateInfo');
- result[0].dsr.should.deep.equal([ 0, 47, 2, 2
]);
+ result[0].dsr.should.deep.equal([ 24, 36, 4, 5
]);
});
});
});
--
To view, visit https://gerrit.wikimedia.org/r/384198
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I7d99c1771fb4d55866d296ecd283202bfc62a8ed
Gerrit-PatchSet: 6
Gerrit-Project: mediawiki/services/parsoid
Gerrit-Branch: master
Gerrit-Owner: Subramanya Sastry <[email protected]>
Gerrit-Reviewer: Arlolra <[email protected]>
Gerrit-Reviewer: C. Scott Ananian <[email protected]>
Gerrit-Reviewer: Legoktm <[email protected]>
Gerrit-Reviewer: Sbailey <[email protected]>
Gerrit-Reviewer: Subramanya Sastry <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
