fix: incorrect lint error position (#16526)
* perform string search using original binding * Add jest test * Improve code for better readability
This commit is contained in:
parent
a706ad3160
commit
16b04f33a6
|
|
@ -44,8 +44,8 @@ describe("getLintAnnotations()", () => {
|
||||||
const { LINT, PARSE } = PropertyEvaluationErrorType;
|
const { LINT, PARSE } = PropertyEvaluationErrorType;
|
||||||
const { ERROR, WARNING } = Severity;
|
const { ERROR, WARNING } = Severity;
|
||||||
it("should return proper annotations", () => {
|
it("should return proper annotations", () => {
|
||||||
const value = `Hello {{ world == test }}`;
|
const value1 = `Hello {{ world == test }}`;
|
||||||
const errors: EvaluationError[] = [
|
const errors1: EvaluationError[] = [
|
||||||
{
|
{
|
||||||
errorType: LINT,
|
errorType: LINT,
|
||||||
raw:
|
raw:
|
||||||
|
|
@ -87,8 +87,8 @@ describe("getLintAnnotations()", () => {
|
||||||
},
|
},
|
||||||
];
|
];
|
||||||
|
|
||||||
const res = getLintAnnotations(value, errors, {});
|
const res1 = getLintAnnotations(value1, errors1, {});
|
||||||
expect(res).toEqual([
|
expect(res1).toEqual([
|
||||||
{
|
{
|
||||||
from: {
|
from: {
|
||||||
line: 0,
|
line: 0,
|
||||||
|
|
@ -126,6 +126,40 @@ describe("getLintAnnotations()", () => {
|
||||||
severity: "warning",
|
severity: "warning",
|
||||||
},
|
},
|
||||||
]);
|
]);
|
||||||
|
|
||||||
|
/// 2
|
||||||
|
const value2 = `hss{{hss}}`;
|
||||||
|
const errors2: EvaluationError[] = [
|
||||||
|
{
|
||||||
|
errorType: LINT,
|
||||||
|
raw:
|
||||||
|
"\n function closedFunction () {\n const result = hss\n return result;\n }\n closedFunction.call(THIS_CONTEXT)\n ",
|
||||||
|
severity: ERROR,
|
||||||
|
errorMessage: "'hss' is not defined.",
|
||||||
|
errorSegment: " const result = hss",
|
||||||
|
originalBinding: "{{hss}}",
|
||||||
|
variables: ["hss", null, null, null],
|
||||||
|
code: "W117",
|
||||||
|
line: 0,
|
||||||
|
ch: 1,
|
||||||
|
},
|
||||||
|
];
|
||||||
|
|
||||||
|
const res2 = getLintAnnotations(value2, errors2, {});
|
||||||
|
expect(res2).toEqual([
|
||||||
|
{
|
||||||
|
from: {
|
||||||
|
line: 0,
|
||||||
|
ch: 5,
|
||||||
|
},
|
||||||
|
to: {
|
||||||
|
line: 0,
|
||||||
|
ch: 8,
|
||||||
|
},
|
||||||
|
message: "'hss' is not defined.",
|
||||||
|
severity: "error",
|
||||||
|
},
|
||||||
|
]);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("should return correct annotation with newline in original binding", () => {
|
it("should return correct annotation with newline in original binding", () => {
|
||||||
|
|
|
||||||
|
|
@ -2,6 +2,7 @@ import { last, isNumber, isEmpty } from "lodash";
|
||||||
import { Annotation, Position } from "codemirror";
|
import { Annotation, Position } from "codemirror";
|
||||||
import {
|
import {
|
||||||
EvaluationError,
|
EvaluationError,
|
||||||
|
isDynamicValue,
|
||||||
PropertyEvaluationErrorType,
|
PropertyEvaluationErrorType,
|
||||||
} from "utils/DynamicBindingUtils";
|
} from "utils/DynamicBindingUtils";
|
||||||
import { Severity } from "entities/AppsmithConsole";
|
import { Severity } from "entities/AppsmithConsole";
|
||||||
|
|
@ -117,6 +118,11 @@ export const getLintAnnotations = (
|
||||||
const annotations: Annotation[] = [];
|
const annotations: Annotation[] = [];
|
||||||
const lintErrors = filterLintErrors(errors, contextData);
|
const lintErrors = filterLintErrors(errors, contextData);
|
||||||
const lines = value.split("\n");
|
const lines = value.split("\n");
|
||||||
|
|
||||||
|
// The binding position of every valid JS Object is constant, so we need not
|
||||||
|
// waste time checking for position of binding.
|
||||||
|
// For JS Objects not starting with the expected "export default" statement, we return early
|
||||||
|
// with a "invalid start statement" lint error
|
||||||
if (
|
if (
|
||||||
isJSObject &&
|
isJSObject &&
|
||||||
!isEmpty(lines) &&
|
!isEmpty(lines) &&
|
||||||
|
|
@ -167,8 +173,19 @@ export const getLintAnnotations = (
|
||||||
for (const bindingLocation of bindingPositions) {
|
for (const bindingLocation of bindingPositions) {
|
||||||
const currentLine = bindingLocation.line + line;
|
const currentLine = bindingLocation.line + line;
|
||||||
const lineContent = lines[currentLine] || "";
|
const lineContent = lines[currentLine] || "";
|
||||||
const currentCh =
|
let currentCh: number;
|
||||||
bindingLocation.line !== currentLine ? ch : bindingLocation.ch + ch;
|
|
||||||
|
// for case where "{{" is in the same line as the lint error
|
||||||
|
if (bindingLocation.line === currentLine) {
|
||||||
|
currentCh =
|
||||||
|
bindingLocation.ch +
|
||||||
|
ch +
|
||||||
|
// Add 2 to account for "{{", if binding is a dynamicValue (NB: JS Objects are dynamicValues without "{{}}")
|
||||||
|
(isDynamicValue(originalBinding) ? 2 : 0);
|
||||||
|
} else {
|
||||||
|
currentCh = ch;
|
||||||
|
}
|
||||||
|
|
||||||
// Jshint counts \t as two characters and codemirror counts it as 1.
|
// Jshint counts \t as two characters and codemirror counts it as 1.
|
||||||
// So we need to subtract number of tabs to get accurate position
|
// So we need to subtract number of tabs to get accurate position
|
||||||
const tabs = lineContent.slice(0, currentCh).match(/\t/g)?.length || 0;
|
const tabs = lineContent.slice(0, currentCh).match(/\t/g)?.length || 0;
|
||||||
|
|
|
||||||
|
|
@ -15,11 +15,7 @@ import {
|
||||||
removeLintErrorsFromEntityProperty,
|
removeLintErrorsFromEntityProperty,
|
||||||
} from "workers/evaluationUtils";
|
} from "workers/evaluationUtils";
|
||||||
|
|
||||||
import {
|
import { getJSToLint, getLintingErrors, pathRequiresLinting } from "./utils";
|
||||||
getJSSnippetToLint,
|
|
||||||
getLintingErrors,
|
|
||||||
pathRequiresLinting,
|
|
||||||
} from "./utils";
|
|
||||||
|
|
||||||
interface LintTreeArgs {
|
interface LintTreeArgs {
|
||||||
unEvalTree: DataTree;
|
unEvalTree: DataTree;
|
||||||
|
|
@ -136,19 +132,21 @@ const lintBindingPath = (
|
||||||
);
|
);
|
||||||
|
|
||||||
if (stringSegments) {
|
if (stringSegments) {
|
||||||
jsSnippets.map((jsSnippet) => {
|
jsSnippets.map((jsSnippet, index) => {
|
||||||
const jsSnippetToLint = getJSSnippetToLint(
|
if (jsSnippet) {
|
||||||
|
const jsSnippetToLint = getJSToLint(entity, jsSnippet, propertyPath);
|
||||||
|
// {{user's code}}
|
||||||
|
const originalBinding = getJSToLint(
|
||||||
entity,
|
entity,
|
||||||
jsSnippet,
|
stringSegments[index],
|
||||||
propertyPath,
|
propertyPath,
|
||||||
);
|
);
|
||||||
if (jsSnippet) {
|
|
||||||
const scriptType = getScriptType(false, false);
|
const scriptType = getScriptType(false, false);
|
||||||
const scriptToLint = getScriptToEval(jsSnippetToLint, scriptType);
|
const scriptToLint = getScriptToEval(jsSnippetToLint, scriptType);
|
||||||
lintErrors = getLintingErrors(
|
lintErrors = getLintingErrors(
|
||||||
scriptToLint,
|
scriptToLint,
|
||||||
globalData,
|
globalData,
|
||||||
jsSnippetToLint,
|
originalBinding,
|
||||||
scriptType,
|
scriptType,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -55,7 +55,8 @@ export const pathRequiresLinting = (
|
||||||
return requiresLinting;
|
return requiresLinting;
|
||||||
};
|
};
|
||||||
|
|
||||||
export const getJSSnippetToLint = (
|
// Removes "export default" statement from js Object
|
||||||
|
export const getJSToLint = (
|
||||||
entity: DataTreeEntity,
|
entity: DataTreeEntity,
|
||||||
snippet: string,
|
snippet: string,
|
||||||
propertyPath: string,
|
propertyPath: string,
|
||||||
|
|
@ -97,6 +98,7 @@ function getEvaluationScriptPosition(scriptType: EvaluationScriptType) {
|
||||||
export const getLintingErrors = (
|
export const getLintingErrors = (
|
||||||
script: string,
|
script: string,
|
||||||
data: Record<string, unknown>,
|
data: Record<string, unknown>,
|
||||||
|
// {{user's code}}
|
||||||
originalBinding: string,
|
originalBinding: string,
|
||||||
scriptType: EvaluationScriptType,
|
scriptType: EvaluationScriptType,
|
||||||
): EvaluationError[] => {
|
): EvaluationError[] => {
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user