From 5ec2ff15b6842f7704274fe3e45904464b1cc226 Mon Sep 17 00:00:00 2001 From: Ayush Pahwa Date: Tue, 19 Nov 2024 15:55:44 +0800 Subject: [PATCH] chore: update logic to calculate length of lint (#36548) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description As part of project to migrate linter to eslint, a small dependency we need to take care of is to update how we calculate the length of the lint to be shown. Today we use an array of variables and calculate their char lengths. With eslint, we directly get the length and hence can be passed down to this function. To ensure backward compatibility till we are still phasing out JSHint, a conditional check is added to the linthelper file. > [!NOTE] > This PR is part of a series of [stacked diffs](https://newsletter.pragmaticengineer.com/p/stacked-diffs) and might have changes from it's parent PRs. Untill the blocking parent PRs are merged, this diff would show more changes than are relevant for this PR. Blocking PRs: - #36543 Fixes #36546 ## Automation /test js sanity ### :mag: Cypress test results > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: 7da61e78bb2dffe41e148aba8a62062234eb59d1 > Cypress dashboard. > Tags: `@tag.JS, @tag.Sanity` > Spec: >
Tue, 19 Nov 2024 07:50:17 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No ## Summary by CodeRabbit - **New Features** - Introduced a new optional property `lintLength` to enhance lint error reporting. - Improved handling of dynamic bindings for better accuracy and responsiveness to changes. - **Bug Fixes** - Enhanced test coverage for lint error annotations to ensure correct behavior with and without `lintLength`. - **Documentation** - Updated comments for clarity regarding new linting logic and error handling. --- .../CodeEditor/lintHelpers.test.ts | 51 +++++++++++++++++++ .../CodeEditor/lintHelpers.ts | 17 ++++--- app/client/src/utils/DynamicBindingUtils.ts | 1 + 3 files changed, 63 insertions(+), 6 deletions(-) diff --git a/app/client/src/components/editorComponents/CodeEditor/lintHelpers.test.ts b/app/client/src/components/editorComponents/CodeEditor/lintHelpers.test.ts index 07c0dc443c..381429411c 100644 --- a/app/client/src/components/editorComponents/CodeEditor/lintHelpers.test.ts +++ b/app/client/src/components/editorComponents/CodeEditor/lintHelpers.test.ts @@ -258,6 +258,57 @@ describe("getLintAnnotations()", () => { }, ]); }); + + it("should use provided lintlength when available", () => { + const value = `{{ variable }}`; + const errors: LintError[] = [ + { + errorType: PropertyEvaluationErrorType.LINT, + raw: "variable", + severity: Severity.WARNING, + errorMessage: { + name: "LintingError", + message: "Lint error message.", + }, + errorSegment: "variable", + originalBinding: "variable", + variables: ["variable"], + code: "W001", + line: 0, + ch: 3, + lintLength: 8, // Provided lint length + }, + ]; + + const annotations = getLintAnnotations(value, errors, {}); + + expect(annotations[0].to?.ch).toBe(13); + }); + + it("should calculate lintlength when not provided", () => { + const value = `{{ variable }}`; + const errors: LintError[] = [ + { + errorType: PropertyEvaluationErrorType.LINT, + raw: "variable", + severity: Severity.WARNING, + errorMessage: { + name: "LintingError", + message: "Lint error message.", + }, + errorSegment: "variable", + originalBinding: "variable", + variables: ["variable"], + code: "W001", + line: 0, + ch: 3, + }, + ]; + + const annotations = getLintAnnotations(value, errors, {}); + + expect(annotations[0].to?.ch).toBe(13); + }); }); describe("getFirstNonEmptyPosition", () => { diff --git a/app/client/src/components/editorComponents/CodeEditor/lintHelpers.ts b/app/client/src/components/editorComponents/CodeEditor/lintHelpers.ts index 65a7e34c27..125e41ebaf 100644 --- a/app/client/src/components/editorComponents/CodeEditor/lintHelpers.ts +++ b/app/client/src/components/editorComponents/CodeEditor/lintHelpers.ts @@ -135,6 +135,7 @@ export const getLintAnnotations = ( code, errorMessage, line, + lintLength, originalBinding, severity, variables, @@ -157,16 +158,20 @@ export const getLintAnnotations = ( }); } - let variableLength = 1; + let calculatedLintLength = 1; + // If lint length is provided, then skip the length calculation logic + if (lintLength && lintLength > 0) { + calculatedLintLength = lintLength; + } // Find the variable with minimal length - if (variables) { + else if (variables) { for (const variable of variables) { if (variable) { - variableLength = - variableLength === 1 + calculatedLintLength = + calculatedLintLength === 1 ? String(variable).length - : Math.min(String(variable).length, variableLength); + : Math.min(String(variable).length, calculatedLintLength); } } } @@ -205,7 +210,7 @@ export const getLintAnnotations = ( }; const to = { line: from.line, - ch: from.ch + variableLength, + ch: from.ch + calculatedLintLength, }; annotations.push({ diff --git a/app/client/src/utils/DynamicBindingUtils.ts b/app/client/src/utils/DynamicBindingUtils.ts index 4e89443bcf..897a674652 100644 --- a/app/client/src/utils/DynamicBindingUtils.ts +++ b/app/client/src/utils/DynamicBindingUtils.ts @@ -436,6 +436,7 @@ export interface LintError extends DataTreeError { line: number; ch: number; originalPath?: string; + lintLength?: number; } export interface DataTreeEvaluationProps {