Better error messages for pattern tests (#2364)
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183
diff --git a/tests/pattern-tests.js b/tests/pattern-tests.js
index beb33c7..b35fdb9 100644
--- a/tests/pattern-tests.js
+++ b/tests/pattern-tests.js
@@ -81,7 +81,7 @@ function testPatterns(Prism) {
BFS(Prism.languages, path => {
const { key, value } = path[path.length - 1];
- let tokenPath = '<languages>';
+ let tokenPath = 'Prism.languages';
for (const { key } of path) {
if (!key) {
// do nothing
@@ -172,11 +172,46 @@ function testPatterns(Prism) {
}
}
+ /**
+ * Returns whether the given element will always at the start of the whole match.
+ *
+ * @param {Element} element
+ * @returns {boolean}
+ */
+ function isFirstMatch(element) {
+ const parent = element.parent;
+ switch (parent.type) {
+ case 'Alternative':
+ // all elements before this element have to of zero length
+ if (!parent.elements.slice(0, parent.elements.indexOf(element)).every(isAlwaysZeroWidth)) {
+ return false;
+ }
+ const grandParent = parent.parent;
+ if (grandParent.type === 'Pattern') {
+ return true;
+ } else {
+ return isFirstMatch(grandParent);
+ }
+
+ case 'Quantifier':
+ if (parent.max >= 2) {
+ return false;
+ } else {
+ return isFirstMatch(parent);
+ }
+
+ default:
+ throw new Error(`Internal error: The given node should not be a '${element.type}'.`);
+ }
+ }
+
it('- should not match the empty string', function () {
forEachPattern(({ pattern, tokenPath }) => {
// test for empty string
- assert.notMatch('', pattern, `Token ${tokenPath}: ${pattern} should not match the empty string.`);
+ assert.notMatch('', pattern, `${tokenPath}: ${pattern} should not match the empty string.\n\n`
+ + `Patterns that do match the empty string can potentially cause infinitely many empty tokens. `
+ + `Make sure that all patterns always consume at least one character.`);
});
});
@@ -187,54 +222,26 @@ function testPatterns(Prism) {
forEachCapturingGroup(ast.pattern, () => { hasCapturingGroup = true; });
if (!hasCapturingGroup) {
- assert.fail(`Token ${tokenPath}: The pattern is set to 'lookbehind: true' but does not have a capturing group.`);
+ assert.fail(`${tokenPath}: The pattern is set to 'lookbehind: true' but does not have a capturing group.\n\n`
+ + `Prism lookbehind groups use the captured text of the first capturing group to simulate a lookbehind. `
+ + `Without a capturing group, a lookbehind is not possible.\n`
+ + `To fix this, either add a capturing group for the lookbehind or remove the 'lookbehind' property.`);
}
}
});
});
it('- should not have lookbehind groups that can be preceded by other some characters', function () {
- /**
- * Returns whether the given element will always match the start of the string.
- *
- * @param {Element} element
- * @returns {boolean}
- */
- function isFirstMatch(element) {
- const parent = element.parent;
- switch (parent.type) {
- case 'Alternative':
- // all elements before this element have to of zero length
- if (!parent.elements.slice(0, parent.elements.indexOf(element)).every(isAlwaysZeroWidth)) {
- return false;
- }
- const grandParent = parent.parent;
- if (grandParent.type === 'Pattern') {
- return true;
- } else {
- return isFirstMatch(grandParent);
- }
-
- case 'Quantifier':
- if (parent.max >= 2) {
- return false;
- } else {
- return isFirstMatch(parent);
- }
-
- default:
- throw new Error(`Internal error: The given node should not be a '${element.type}'.`);
- }
- }
-
forEachPattern(({ ast, tokenPath, lookbehind }) => {
if (!lookbehind) {
return;
}
forEachCapturingGroup(ast.pattern, ({ group, number }) => {
if (number === 1 && !isFirstMatch(group)) {
- assert.fail(`Token ${tokenPath}: `
- + `The lookbehind group (if matched) always has to be at index 0 relative to the whole match.`);
+ assert.fail(`${tokenPath}: The lookbehind group ${group.raw} might be preceded by some characters.\n\n`
+ + `Prism assumes that the lookbehind group, if captured, is the first thing matched by the regex. `
+ + `If characters might precede the lookbehind group (e.g. /a?(b)c/), then Prism cannot correctly apply the lookbehind correctly in all cases.\n`
+ + `To fix this, either remove the preceding characters or include them in the lookbehind group.`);
}
});
});
@@ -249,9 +256,9 @@ function testPatterns(Prism) {
if (number === 1 && isAlwaysZeroWidth(group)) {
const groupContent = group.raw.substr(1, group.raw.length - 2);
const replacement = group.alternatives.length === 1 ? groupContent : `(?:${groupContent})`;
- reportError(`Token ${tokenPath}: The lookbehind group ${group.raw} does not consume characters. `
- + `Therefor it is not necessary to use a lookbehind group. `
- + `Replacing the lookbehind group with: ${replacement}`);
+ reportError(`${tokenPath}: The lookbehind group ${group.raw} does not consume characters.\n\n`
+ + `Therefor it is not necessary to use a lookbehind group.\n`
+ + `To fix this, replace the lookbehind group with ${replacement} and remove the 'lookbehind' property.`);
}
});
});
@@ -262,7 +269,22 @@ function testPatterns(Prism) {
forEachCapturingGroup(ast.pattern, ({ group, number }) => {
const isLookbehindGroup = lookbehind && number === 1;
if (group.references.length === 0 && !isLookbehindGroup) {
- reportError(`Token ${tokenPath}: Unused capturing group ${group.raw}. All capturing groups have to be either referenced or used as a Prism lookbehind group.`);
+ const fixes = [];
+ fixes.push(`Make this group a non-capturing group ('(?:...)' instead of '(...)'). (It's usually this option.)`);
+ fixes.push(`Reference this group with a backreference (use '\\${number}' for this).`);
+ if (number === 1 && !lookbehind) {
+ if (isFirstMatch(group)) {
+ fixes.push(`Add a 'lookbehind: true' declaration.`);
+ } else {
+ fixes.push(`Add a 'lookbehind: true' declaration. (This group is not a valid lookbehind group because it can be preceded by some characters.)`);
+ }
+ }
+
+ reportError(`${tokenPath}: Unused capturing group ${group.raw}.\n\n`
+ + `Unused capturing groups generally degrade the performance of regular expressions. `
+ + `They might also be a sign that a backreference is incorrect or that a 'lookbehind: true' declaration in missing.\n`
+ + `To fix this, do one of the following:\n`
+ + fixes.map(f => '- ' + f).join('\n'));
}
});
});
@@ -272,7 +294,8 @@ function testPatterns(Prism) {
const niceName = /^[a-z][a-z\d]*(?:[-_][a-z\d]+)*$/;
function testName(name, desc = 'token name') {
if (!niceName.test(name)) {
- assert.fail(`The ${desc} '${name}' does not match ${niceName}`);
+ assert.fail(`The ${desc} '${name}' does not match ${niceName}.\n\n`
+ + `To fix this, choose a name that matches the above regular expression.`);
}
}
@@ -305,7 +328,10 @@ function testPatterns(Prism) {
visitRegExpAST(ast.pattern, {
onCharacterEnter(node) {
if (/^\\(?:[1-9]|\d{2,})$/.test(node.raw)) {
- reportError(`Token ${tokenPath}: Octal escape ${node.raw}.`);
+ reportError(`${tokenPath}: Octal escape ${node.raw}.\n\n`
+ + `Octal escapes can be confused with backreferences, so please do not use them.\n`
+ + `To fix this, use a different escape method. `
+ + `Note that this could also be an invalid backreference, so be sure to carefully analyse the pattern.`);
}
}
});