Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FES: Handle multiple nearest matches for a misspelt symbol #4719

Merged
merged 1 commit into from
Jul 31, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 86 additions & 35 deletions src/core/friendly_errors/fes_core.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,14 136,41 @@ if (typeof IS_MINIFIED !== 'undefined') {
);
};

/**
* Takes a message and a p5 function func, and adds a link pointing to
* the reference documentation of func at the end of the message
*
* @method mapToReference
* @private
* @param {String} message the words to be said
* @param {String} [func] the name of the function to link
*
* @returns {String}
*/
const mapToReference = (message, func) => {
let msgWithReference = '';
if (func == null || func.substring(0, 4) === 'load') {
msgWithReference = message;
} else {
const methodParts = func.split('.');
const referenceSection =
methodParts.length > 1 ? `${methodParts[0]}.${methodParts[1]}` : 'p5';

const funcName =
methodParts.length === 1 ? func : methodParts.slice(2).join('/');
msgWithReference = `${message} (http://p5js.org/reference/#/${referenceSection}/${funcName})`;
}
return msgWithReference;
};

/**
* Prints out a fancy, colorful message to the console log
*
* @method report
* @private
* @param {String} message the words to be said
* @param {String} func the name of the function to link
* @param {Number|String} color CSS color string or error type
* @param {String} [func] the name of the function to link
* @param {Number|String} [color] CSS color string or error type
*
* @return console logs
*/
Expand All @@ -164,22 191,11 @@ if (typeof IS_MINIFIED !== 'undefined') {
color = typeColors[color];
}

let prefixedMsg;
// Add a link to the reference docs of func at the end of the message
message = mapToReference(message, func);
let style = [`color: ${color}`, 'font-family: Arial', 'font-size: larger'];
if (func == null || func.substring(0, 4) === 'load') {
prefixedMsg = translator('fes.pre', { message });
} else {
const methodParts = func.split('.');
const referenceSection =
methodParts.length > 1 ? `${methodParts[0]}.${methodParts[1]}` : 'p5';

const funcName =
methodParts.length === 1 ? func : methodParts.slice(2).join('/');
const prefixedMsg = translator('fes.pre', { message });

prefixedMsg = translator('fes.pre', {
message: `${message} (http://p5js.org/reference/#/${referenceSection}/${funcName})`
});
}
if (ENABLE_FES_STYLING) {
log('%c' prefixedMsg, style.join(';'));
} else {
Expand All @@ -193,7 209,7 @@ if (typeof IS_MINIFIED !== 'undefined') {
* @method _friendlyError
* @private
* @param {Number} message message to be printed
* @param {String} method name of method
* @param {String} [method] name of method
* @param {Number|String} [color] CSS color string or error type (Optional)
*/
p5._friendlyError = function(message, method, color) {
Expand Down Expand Up @@ -304,25 320,27 @@ if (typeof IS_MINIFIED !== 'undefined') {
defineMisusedAtTopLevelCode();
}

let min = 999999,
minIndex = 0;
const distanceMap = {};
let min = 999999;
// compute the levenshtein distance for the symbol against all known
// public p5 properties. Find the property with the minimum distance
misusedAtTopLevelCode.forEach((symbol, idx) => {
misusedAtTopLevelCode.forEach(symbol => {
let dist = computeEditDistance(errSym, symbol.name);
if (dist < min) {
min = dist;
minIndex = idx;
}
if (distanceMap[dist]) distanceMap[dist].push(symbol);
else distanceMap[dist] = [symbol];

if (dist < min) min = dist;
});

// if the closest match has more "distance" than the max allowed threshold
if (min > Math.min(EDIT_DIST_THRESHOLD, errSym.length)) return false;

let symbol = misusedAtTopLevelCode[minIndex];

// Show a message only if the caught symbol and the matched property name
// differ in their name ( either letter difference or difference of case )
if (errSym !== symbol.name) {
const matchedSymbols = distanceMap[min].filter(
symbol => symbol.name !== errSym
);
if (matchedSymbols.length !== 0) {
const parsed = p5._getErrorStackParser().parse(error);
let locationObj;
if (
Expand All @@ -336,18 354,51 @@ if (typeof IS_MINIFIED !== 'undefined') {
location: `${parsed[0].fileName}:${parsed[0].lineNumber}:${
parsed[0].columnNumber
}`,
file: parsed[0].fileName,
file: parsed[0].fileName.split('/').slice(-1),
line: parsed[0].lineNumber
};
}
const msg = translator('fes.misspelling', {
name: errSym,
actualName: symbol.name,
type: symbol.type,
location: locationObj ? translator('fes.location', locationObj) : ''
});

p5._friendlyError(msg, symbol.name);
let msg;
if (matchedSymbols.length === 1) {
// To be used when there is only one closest match. The count parameter
// allows i18n to pick between the keys "fes.misspelling" and
// "fes.misspelling__plural"
msg = translator('fes.misspelling', {
name: errSym,
actualName: matchedSymbols[0].name,
type: matchedSymbols[0].type,
location: locationObj ? translator('fes.location', locationObj) : '',
count: matchedSymbols.length
});
} else {
// To be used when there are multiple closest matches. Gives each
// suggestion on its own line, the function name followed by a link to
// reference documentation
const suggestions = matchedSymbols
.map(symbol => {
const message =
'▶️ ' symbol.name (symbol.type === 'function' ? '()' : '');
return mapToReference(message, symbol.name);
})
.join('\n');

msg = translator('fes.misspelling', {
name: errSym,
suggestions: suggestions,
location: locationObj ? translator('fes.location', locationObj) : '',
count: matchedSymbols.length
});
}

// If there is only one closest match, tell _friendlyError to also add
// a link to the reference documentation. In case of multiple matches,
// this is already done in the suggestions variable, one link for each
// suggestion.
p5._friendlyError(
msg,
matchedSymbols.length === 1 ? matchedSymbols[0].name : undefined
);
return true;
}
return false;
Expand Down
31 changes: 25 additions & 6 deletions test/unit/core/error_helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -388,24 388,43 @@ suite('Error Helpers', function() {
});

testUnMinified('detects capitalization mistakes', function() {
const logMsg = help(new ReferenceError('MouseX is not defined'));
assert.match(
help(new ReferenceError('MouseX is not defined')),
/It seems that you may have accidently written MouseX instead of mouseX/
logMsg,
/It seems that you may have accidently written "MouseX"/
);
assert.match(logMsg, /mouseX/);
});

testUnMinified('detects spelling mistakes', function() {
const logMsg = help(new ReferenceError('colour is not defined'));
assert.match(
help(new ReferenceError('colour is not defined')),
/It seems that you may have accidently written colour instead of color/
logMsg,
/It seems that you may have accidently written "colour"/
);
assert.match(logMsg, /color/);
});

testUnMinified(
'can give more than one closest matches, if applicable',
function() {
const logMsg = help(new ReferenceError('strok is not defined'));
assert.match(
logMsg,
/It seems that you may have accidently written "strok"/
);
assert.match(logMsg, /stroke/);
assert.match(logMsg, /STROKE/);
}
);

testUnMinified('detects spelling captialization mistakes', function() {
const logMsg = help(new ReferenceError('RandomGossian is not defined'));
assert.match(
help(new ReferenceError('RandomGossian is not defined')),
/It seems that you may have accidently written RandomGossian instead of randomGaussian/
logMsg,
/It seems that you may have accidently written "RandomGossian"/
);
assert.match(logMsg, /randomGaussian/);
});
});

Expand Down
3 changes: 2 additions & 1 deletion translations/en/translation.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 37,8 @@
},
"libraryError": "An error with message \"{{error}}\" occured inside the p5js library when {{func}} was called {{location}}\n\nIf not stated otherwise, it might be an issue with the arguments passed to {{func}}.",
"location": "(on line {{line}} in {{file}} [{{location}}])",
"misspelling": "It seems that you may have accidently written {{name}} instead of {{actualName}} {{location}}.\n\nPlease correct it to {{actualName}} if you wish to use the {{type}} from p5.js",
"misspelling": "It seems that you may have accidently written \"{{name}}\" instead of \"{{actualName}}\" {{location}}.\n\nPlease correct it to {{actualName}} if you wish to use the {{type}} from p5.js",
"misspelling_plural": "It seems that you may have accidently written \"{{name}}\" {{location}}.\n\nYou may have meant one of the following:\n{{suggestions}}",
"misusedTopLevel": "Did you just try to use p5.js's {{symbolName}} {{symbolType}}? If so, you may want to move it into your sketch's setup() function.\n\nFor more details, see: {{link}}",
"positions": {
"p_1": "first",
Expand Down
1 change: 1 addition & 0 deletions translations/es/translation.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 38,7 @@
"libraryError": "",
"location": "",
"misspelling": "",
"misspelling_plural": "",
"misusedTopLevel": "",
"positions": {
"p_1": "",
Expand Down