diff --git a/app/client/package.json b/app/client/package.json index 1e82d2e267..abd7686861 100644 --- a/app/client/package.json +++ b/app/client/package.json @@ -81,7 +81,7 @@ "interweave-autolink": "^4.4.2", "js-beautify": "^1.14.0", "js-sha256": "^0.9.0", - "jshint": "^2.12.0", + "jshint": "^2.13.1", "json-fn": "^1.1.1", "lint-staged": "^9.2.5", "localforage": "^1.7.3", diff --git a/app/client/src/components/editorComponents/CodeEditor/index.tsx b/app/client/src/components/editorComponents/CodeEditor/index.tsx index 3aeb74d1ec..0b01775fc8 100644 --- a/app/client/src/components/editorComponents/CodeEditor/index.tsx +++ b/app/client/src/components/editorComponents/CodeEditor/index.tsx @@ -268,6 +268,9 @@ class CodeEditor extends Component { this.props.showLightningMenu, this.props.additionalDynamicData, ); + if (getFeatureFlags().LINTING) { + this.lintCode(editor); + } }; // Finally create the Codemirror editor @@ -463,10 +466,10 @@ class CodeEditor extends Component { } }; - lintCode() { + lintCode(editor: CodeMirror.Editor) { const { dataTreePath, dynamicData } = this.props; - if (!dataTreePath || !this.updateLintingCallback) { + if (!dataTreePath || !this.updateLintingCallback || !editor) { return; } @@ -478,11 +481,9 @@ class CodeEditor extends Component { let annotations: Annotation[] = []; - if (this.editor) { - annotations = getLintAnnotations(this.editor.getValue(), errors); - } + annotations = getLintAnnotations(editor.getValue(), errors); - this.updateLintingCallback(this.editor, annotations); + this.updateLintingCallback(editor, annotations); } static updateMarkings = ( @@ -586,7 +587,7 @@ class CodeEditor extends Component { /* Evaluation results for snippet snippets */ if (getFeatureFlags().LINTING) { - this.lintCode(); + this.lintCode(this.editor); } const showEvaluatedValue = diff --git a/app/client/src/components/editorComponents/CodeEditor/lintHelpers.test.ts b/app/client/src/components/editorComponents/CodeEditor/lintHelpers.test.ts index e242c59bbe..5a9011e942 100644 --- a/app/client/src/components/editorComponents/CodeEditor/lintHelpers.test.ts +++ b/app/client/src/components/editorComponents/CodeEditor/lintHelpers.test.ts @@ -39,70 +39,49 @@ describe("getKeyPositionsInString()", () => { }); describe("getLintAnnotations()", () => { - const { LINT, PARSE } = PropertyEvaluationErrorType; - const { ERROR, WARNING } = Severity; + const { LINT } = PropertyEvaluationErrorType; + const { WARNING } = Severity; it("should return proper annotations", () => { - const value = `Hello {{ world == test }}\n {{text}}`; + const value = `Hello {{ world == test }}`; const errors: EvaluationError[] = [ { - errorMessage: "Expected '===' and instead saw '=='.", - severity: WARNING, - raw: - "\n function closedFunction () {\n const result = world == test \n return result;\n }\n closedFunction()\n ", errorType: LINT, - originalBinding: "world == test ", - errorSegment: " const result = world == test ", + raw: + "\n function closedFunction () {\n const result = world == test \n return result;\n }\n closedFunction()\n ", + severity: WARNING, + errorMessage: "Expected '===' and instead saw '=='.", + errorSegment: " const result = world == test ", + originalBinding: " world == test ", variables: ["===", "==", null, null], code: "W116", + line: 0, + ch: 8, }, { errorType: LINT, raw: - "\n function closedFunction () {\n const result = world == test \n return result;\n }\n closedFunction()\n ", + "\n function closedFunction () {\n const result = world == test \n return result;\n }\n closedFunction()\n ", severity: WARNING, errorMessage: "'world' is not defined.", - errorSegment: " const result = world == test ", - originalBinding: "world == test ", + errorSegment: " const result = world == test ", + originalBinding: " world == test ", variables: ["world", null, null, null], code: "W117", + line: 0, + ch: 2, }, { - errorType: LINT, - raw: - "\n function closedFunction () {\n const result = world == test \n return result;\n }\n closedFunction()\n ", - severity: WARNING, errorMessage: "'test' is not defined.", - errorSegment: " const result = world == test ", - originalBinding: "world == test ", + severity: WARNING, + raw: + "\n function closedFunction () {\n const result = world == test \n return result;\n }\n closedFunction()\n ", + errorType: LINT, + originalBinding: " world == test ", + errorSegment: " const result = world == test ", variables: ["test", null, null, null], code: "W117", - }, - { - errorType: PARSE, - raw: - "\n function closedFunction () {\n const result = world == test \n return result;\n }\n closedFunction()\n ", - severity: ERROR, - errorMessage: "ReferenceError: world is not defined", - originalBinding: " world == test ", - }, - { - errorMessage: "'text' is not defined.", - severity: WARNING, - raw: - "\n function closedFunction () {\n const result = text\n return result;\n }\n closedFunction()\n ", - errorType: LINT, - originalBinding: "text", - errorSegment: " const result = text", - variables: ["text", null, null, null], - code: "W117", - }, - { - errorMessage: "ReferenceError: text is not defined", - severity: WARNING, - raw: - "\n function closedFunction () {\n const result = text\n return result;\n }\n closedFunction()\n ", - errorType: PARSE, - originalBinding: "text", + line: 0, + ch: 11, }, ]; @@ -144,18 +123,6 @@ describe("getLintAnnotations()", () => { message: "'test' is not defined.", severity: "warning", }, - { - from: { - line: 1, - ch: 4, - }, - to: { - line: 1, - ch: 8, - }, - message: "'text' is not defined.", - severity: "warning", - }, ]); }); }); diff --git a/app/client/src/components/editorComponents/CodeEditor/lintHelpers.ts b/app/client/src/components/editorComponents/CodeEditor/lintHelpers.ts index 551248ae93..0cd051ccd0 100644 --- a/app/client/src/components/editorComponents/CodeEditor/lintHelpers.ts +++ b/app/client/src/components/editorComponents/CodeEditor/lintHelpers.ts @@ -1,4 +1,4 @@ -import _ from "lodash"; +import { last, isNumber } from "lodash"; import { Annotation, Position } from "codemirror"; import { EvaluationError, @@ -44,7 +44,7 @@ export const getKeyPositionInString = ( for (const index of indices) { const substr = str.substr(0, index); const substrLines = substr.split("\n"); - const ch = _.last(substrLines)?.length || 0; + const ch = last(substrLines)?.length || 0; const line = substrLines.length - 1; positions.push({ line, ch }); @@ -63,68 +63,65 @@ export const getLintAnnotations = ( const lintErrors = errors.filter( (error) => error.errorType === PropertyEvaluationErrorType.LINT, ); - + const lines = value.split("\n"); lintErrors.forEach((error) => { - const { errorMessage, originalBinding, severity, variables } = error; + const { + ch, + errorMessage, + line, + originalBinding, + severity, + variables, + } = error; if (!originalBinding) { return annotations; } - // We find the location of binding in the editor value and then - // we find locations of jshint variables (a, b, c, d) in the binding and highlight them + let variableLength = 1; + // Find the variable with minimal length + if (variables) { + for (const variable of variables) { + if (variable) { + variableLength = + variableLength === 1 + ? variable.length + : Math.min(variable.length, variableLength); + } + } + } + const bindingPositions = getKeyPositionInString(value, originalBinding); - for (const bindingLocation of bindingPositions) { - if (variables?.filter((v) => v).length) { - for (let variable of variables) { - if (typeof variable === "number") { - variable = variable.toString(); - } - if (variable && originalBinding.includes(variable)) { - const variableLocations = getKeyPositionInString( - originalBinding, - variable, - ); + if (isNumber(line) && isNumber(ch)) { + for (const bindingLocation of bindingPositions) { + const currentLine = bindingLocation.line + line; + const lineContent = lines[currentLine] || ""; + const currentCh = originalBinding.includes("\n") + ? ch + : bindingLocation.ch + ch; + // Jshint counts \t as two characters and codemirror counts it as 1. + // So we need to subtract number of tabs to get accurate position + const tabs = lineContent.substr(0, currentCh).match(/\t/g)?.length || 0; - for (const variableLocation of variableLocations) { - const from = { - line: bindingLocation.line + variableLocation.line, - // if the binding is a multiline function we need to - // use jshint variable position as the starting point - ch: - variableLocation.line > 0 - ? variableLocation.ch - : variableLocation.ch + bindingLocation.ch, - }; - - const to = { - line: from.line, - ch: from.ch + variable.length, - }; - - const annotation = { - from, - to, - message: errorMessage, - severity, - }; - - annotations.push(annotation); - } - } - } - } else { - const from = bindingLocation; - const to = { line: from.line, ch: from.ch + 3 }; - const annotation = { + const from = { + line: currentLine, + ch: currentCh - tabs - 1, + }; + const to = { + line: from.line, + ch: from.ch + variableLength, + }; + annotations.push({ from, to, message: errorMessage, severity, - }; - annotations.push(annotation); + }); } + } else { + // Don't show linting errors if code has parsing errors + return []; } }); return annotations; diff --git a/app/client/src/sagas/PostEvaluationSagas.ts b/app/client/src/sagas/PostEvaluationSagas.ts index 0f08804e54..d89cdd8c76 100644 --- a/app/client/src/sagas/PostEvaluationSagas.ts +++ b/app/client/src/sagas/PostEvaluationSagas.ts @@ -130,10 +130,12 @@ function logLatestEvalPropertyErrors( // Reformatting eval errors here to a format usable by the debugger const errorMessages = errors.map((e) => { // Error format required for the debugger - return { + const formattedError = { message: e.errorMessage, type: e.errorType, }; + + return formattedError; }); const analyticsData = isWidget(entity) diff --git a/app/client/src/utils/DynamicBindingUtils.ts b/app/client/src/utils/DynamicBindingUtils.ts index 6f01f417b8..b4d40de546 100644 --- a/app/client/src/utils/DynamicBindingUtils.ts +++ b/app/client/src/utils/DynamicBindingUtils.ts @@ -353,8 +353,10 @@ export type EvaluationError = { severity: Severity.WARNING | Severity.ERROR; errorSegment?: string; originalBinding?: string; - variables?: (string | undefined | null | number)[]; + variables?: (string | undefined | null)[]; code?: string; + line?: number; + ch?: number; }; export interface DataTreeEvaluationProps { diff --git a/app/client/src/workers/evaluate.test.ts b/app/client/src/workers/evaluate.test.ts index 9eb634ed61..9e2e67d54a 100644 --- a/app/client/src/workers/evaluate.test.ts +++ b/app/client/src/workers/evaluate.test.ts @@ -47,10 +47,12 @@ describe("evaluate", () => { triggers: [], errors: [ { + ch: 1, code: "W117", errorMessage: "'wrongJS' is not defined.", errorSegment: " const result = wrongJS", errorType: "LINT", + line: 0, raw: ` function closedFunction () { const result = wrongJS @@ -58,7 +60,7 @@ describe("evaluate", () => { } closedFunction() `, - severity: "warning", + severity: "error", originalBinding: "wrongJS", variables: ["wrongJS", undefined, undefined, undefined], }, diff --git a/app/client/src/workers/evaluate.ts b/app/client/src/workers/evaluate.ts index daa1ef4d86..4bee68c7a4 100644 --- a/app/client/src/workers/evaluate.ts +++ b/app/client/src/workers/evaluate.ts @@ -8,9 +8,10 @@ import { import unescapeJS from "unescape-js"; import { JSHINT as jshint } from "jshint"; import { Severity } from "entities/AppsmithConsole"; +import { Position } from "codemirror"; import { AppsmithPromise, enhanceDataTreeWithFunctions } from "./Actions"; import { ActionDescription } from "entities/DataTree/actionTriggers"; -import { isEmpty } from "lodash"; +import { isEmpty, last } from "lodash"; export type EvalResult = { result: any; @@ -24,34 +25,44 @@ export enum EvaluationScriptType { TRIGGERS = "TRIGGERS", } -const evaluationScripts: Record< - EvaluationScriptType, - (script: string) => string -> = { - [EvaluationScriptType.EXPRESSION]: (script: string) => ` +const evaluationScriptsPos: Record = { + [EvaluationScriptType.EXPRESSION]: ` function closedFunction () { - const result = ${script} + const result = <