Merge pull request #214 from KxSystems/ecmel-linter

[LS] refactoring for linter
KxSystems · Dec 19, 2023 · 9b3f0f5 · 9b3f0f5
2 parents 10c40dd + 20c341d
commit 9b3f0f5
Show file tree

Hide file tree

Showing 21 changed files with 1,328 additions and 816 deletions.
diff --git a/package.json b/package.json
@@ -179,6 +179,11 @@
           "type": "boolean",
           "description": "Hide subscribe for registration notification",
           "default": false
+        },
+        "kdb.linting": {
+          "type": "boolean",
+          "description": "Enable linting for q files",
+          "default": true
         }
       }
     },

diff --git a/server/src/linter/assign.ts b/server/src/linter/assign.ts
@@ -11,67 +11,70 @@
  * specific language governing permissions and limitations under the License.
  */
 
-import { Entity, EntityType, QAst, getNameScope, isLiteral } from "../parser";
+import { Token, TokenType, QAst, scope } from "../parser";
 
-export function assignReservedWord({ assign }: QAst): Entity[] {
-  return assign.filter((entity) => entity.type === EntityType.KEYWORD);
+export function assignReservedWord({ assign }: QAst): Token[] {
+  return assign.filter((entity) => entity.type === TokenType.KEYWORD);
 }
 
-export function invalidAssign({ assign }: QAst): Entity[] {
-  return assign.filter((entity) => isLiteral(entity));
+export function invalidAssign({ assign }: QAst): Token[] {
+  return assign.filter((entity) => entity.type === TokenType.LITERAL);
 }
 
-export function declaredAfterUse({ script, assign }: QAst): Entity[] {
-  return script.filter((entity, index) => {
-    if (entity.type === EntityType.IDENTIFIER) {
-      const declared = assign.find(
+export function unusedParam({ script, assign }: QAst): Token[] {
+  assign = assign.filter(
+    (token) => token.type === TokenType.IDENTIFIER && token.tag === "ARGUMENT"
+  );
+
+  script = script.filter(
+    (token) =>
+      token.tag !== "ASSIGNED" &&
+      token.tag !== "ARGUMENT" &&
+      token.type === TokenType.IDENTIFIER &&
+      assign.find((symbol) => symbol.image === token.image)
+  );
+
+  assign = assign.filter(
+    (token) =>
+      !script.find(
         (symbol) =>
-          getNameScope(symbol) === getNameScope(entity) &&
-          symbol.image === entity.image
-      );
-      return declared && script.indexOf(declared) > index;
-    }
-    return false;
-  });
-}
+          symbol.image === token.image && scope(symbol) === scope(token)
+      )
+  );
 
-export function unusedParam({ script, assign }: QAst): Entity[] {
-  return assign
-    .filter(
-      (entity) =>
-        entity.type === EntityType.IDENTIFIER &&
-        entity.scope?.type === EntityType.LBRACKET
-    )
-    .filter(
-      (entity) =>
-        !script.find(
-          (symbol) =>
-            symbol !== entity &&
-            symbol.image === entity.image &&
-            symbol.type === EntityType.IDENTIFIER &&
-            getNameScope(symbol) === getNameScope(entity)
-        )
-    );
+  return assign;
 }
 
-export function unusedVar({ script, assign }: QAst): Entity[] {
-  const locals = assign.filter((entity) => getNameScope(entity));
+export function unusedVar({ script, assign }: QAst): Token[] {
+  const locals = assign.filter(
+    (token) => token.type === TokenType.IDENTIFIER && scope(token)
+  );
+
+  assign = assign.filter(
+    (token) => token.type === TokenType.IDENTIFIER && token.tag === "ASSIGNED"
+  );
+
+  script = script.filter(
+    (token) =>
+      token.tag !== "ASSIGNED" &&
+      token.tag !== "ARGUMENT" &&
+      token.type === TokenType.IDENTIFIER &&
+      assign.find((symbol) => symbol.image === token.image)
+  );
+
+  assign = assign.filter(
+    (token) =>
+      !script.find(
+        (symbol) =>
+          symbol.image === token.image &&
+          (scope(symbol) === scope(token) ||
+            (!scope(token) &&
+              !locals.find(
+                (local) =>
+                  local.image === token.image && scope(local) === scope(symbol)
+              )))
+      )
+  );
 
-  return assign
-    .filter(
-      (entity) =>
-        entity.type === EntityType.IDENTIFIER &&
-        entity.scope?.type !== EntityType.LBRACKET
-    )
-    .filter(
-      (entity) =>
-        !script.find(
-          (symbol) =>
-            symbol !== entity &&
-            symbol.image === entity.image &&
-            symbol.type === EntityType.IDENTIFIER &&
-            (getNameScope(symbol) === getNameScope(entity) ||
-              !locals.find((local) => local.image === symbol.image))
-        )
-    );
+  return assign;
 }
diff --git a/server/src/linter/index.ts b/server/src/linter/index.ts
@@ -11,27 +11,25 @@
  * specific language governing permissions and limitations under the License.
  */
 
-import { Entity, QAst } from "../parser";
+import { Token, QAst } from "../parser";
 import { RuleSeverity, Rules } from "./rules";
 
 const enabled = [
   "ASSIGN_RESERVED_WORD",
+  "DEPRECATED_DATETIME",
   "INVALID_ASSIGN",
-  //"DECLARED_AFTER_USE",
-  "UNUSED_PARAM",
-  "UNUSED_VAR",
-  "LINE_LENGTH",
   "TOO_MANY_CONSTANTS",
   "TOO_MANY_GLOBALS",
   "TOO_MANY_LOCALS",
-  "DEPRECATED_DATETIME",
+  "UNUSED_PARAM",
+  "UNUSED_VAR",
 ];
 
 export interface LintResult {
   name: string;
   message: string;
   severity: RuleSeverity;
-  problems: Entity[];
+  problems: Token[];
 }
 
 export function lint(ast: QAst) {

diff --git a/server/src/linter/limit.ts b/server/src/linter/limit.ts
@@ -11,51 +11,32 @@
  * specific language governing permissions and limitations under the License.
  */
 
-import { Entity, EntityType, QAst, getNameScope } from "../parser";
+import { Token, TokenType, QAst, scope } from "../parser";
 
-const DEFAULT_MAX_LINE_LENGTH = 200;
 const DEFAULT_MAX_LOCALS = 110;
 const DEFAULT_MAX_GLOBALS = 110;
 const DEFAULT_MAX_CONSTANTS = 239;
 
-export function lineLength({ script }: QAst): Entity[] {
-  const problems: Entity[] = [];
-
-  const symbols = script.filter(
-    (entity) => entity.type === EntityType.ENDOFLINE
-  );
-
-  for (let i = 0; i < symbols.length; i++) {
-    const start = i === 0 ? 0 : symbols[i - 1].endOffset;
-
-    if (symbols[i].endOffset - start > DEFAULT_MAX_LINE_LENGTH + 1) {
-      problems.push(symbols[i]);
-    }
-  }
-
-  return problems;
-}
-
-export function tooManyConstants({ script }: QAst): Entity[] {
-  const counts = new Map<Entity, number>();
+export function tooManyConstants({ script }: QAst): Token[] {
+  const counts = new Map<Token, number>();
 
   script
-    .filter((entity) => getNameScope(entity))
+    .filter((entity) => scope(entity))
     .forEach((entity) => {
-      if (entity.type === EntityType.IDENTIFIER) {
-        const scope = getNameScope(entity);
-        if (scope) {
-          let count = counts.get(scope);
+      if (entity.type === TokenType.IDENTIFIER) {
+        const scoped = scope(entity);
+        if (scoped) {
+          let count = counts.get(scoped);
           if (!count) {
             count = 0;
           }
           count++;
-          counts.set(scope, count);
+          counts.set(scoped, count);
         }
       }
     });
 
-  const problems: Entity[] = [];
+  const problems: Token[] = [];
 
   for (const entry of counts.entries()) {
     if (entry[1] > DEFAULT_MAX_CONSTANTS) {
@@ -66,28 +47,28 @@ export function tooManyConstants({ script }: QAst): Entity[] {
   return problems;
 }
 
-export function tooManyGlobals({ script, assign }: QAst): Entity[] {
-  const counts = new Map<Entity, number>();
+export function tooManyGlobals({ script, assign }: QAst): Token[] {
+  const counts = new Map<Token, number>();
 
-  const globals = assign.filter((entity) => !getNameScope(entity));
+  const globals = assign.filter((entity) => !scope(entity));
 
   script
-    .filter((entity) => getNameScope(entity))
+    .filter((entity) => scope(entity))
     .forEach((entity) => {
-      const scope = getNameScope(entity);
-      if (scope) {
-        let count = counts.get(scope);
+      const scoped = scope(entity);
+      if (scoped) {
+        let count = counts.get(scoped);
         if (!count) {
           count = 0;
         }
         if (globals.find((symbol) => symbol.image === entity.image)) {
           count++;
         }
-        counts.set(scope, count);
+        counts.set(scoped, count);
       }
     });
 
-  const problems: Entity[] = [];
+  const problems: Token[] = [];
 
   for (const entry of counts.entries()) {
     if (entry[1] > DEFAULT_MAX_GLOBALS) {
@@ -98,31 +79,31 @@ export function tooManyGlobals({ script, assign }: QAst): Entity[] {
   return problems;
 }
 
-export function tooManyLocals({ assign }: QAst): Entity[] {
-  const counts = new Map<Entity, number>();
+export function tooManyLocals({ assign }: QAst): Token[] {
+  const counts = new Map<Token, number>();
 
   assign
-    .filter((entity) => getNameScope(entity))
+    .filter((entity) => scope(entity))
     .forEach((entity) => {
-      const scope = getNameScope(entity);
-      if (scope) {
-        let count = counts.get(scope);
+      const scoped = scope(entity);
+      if (scoped) {
+        let count = counts.get(scoped);
         if (!count) {
           count = 0;
         }
         if (
           assign.find(
             (symbol) =>
-              getNameScope(symbol) === scope && entity.image === symbol.image
+              scope(symbol) === scoped && entity.image === symbol.image
           )
         ) {
           count++;
         }
-        counts.set(scope, count);
+        counts.set(scoped, count);
       }
     });
 
-  const problems: Entity[] = [];
+  const problems: Token[] = [];
 
   for (const entry of counts.entries()) {
     if (entry[1] > DEFAULT_MAX_LOCALS) {

diff --git a/server/src/linter/other.ts b/server/src/linter/other.ts
@@ -11,8 +11,8 @@
  * specific language governing permissions and limitations under the License.
  */
 
-import { Entity, EntityType, QAst } from "../parser";
+import { Token, QAst } from "../parser";
 
-export function deprecatedDatetime({ script }: QAst): Entity[] {
-  return script.filter((entity) => entity.type === EntityType.DATETIME_LITERAL);
+export function deprecatedDatetime({ script }: QAst): Token[] {
+  return script.filter((entity) => entity.name === "DateTimeLiteral");
 }
diff --git a/server/src/linter/rules.ts b/server/src/linter/rules.ts
@@ -11,33 +11,28 @@
  * specific language governing permissions and limitations under the License.
  */
 
-import { Entity, QAst } from "../parser";
+import { Token, QAst } from "../parser";
 import {
   assignReservedWord,
-  declaredAfterUse,
   invalidAssign,
   unusedParam,
   unusedVar,
 } from "./assign";
-import {
-  lineLength,
-  tooManyConstants,
-  tooManyGlobals,
-  tooManyLocals,
-} from "./limit";
+import { tooManyConstants, tooManyGlobals, tooManyLocals } from "./limit";
 import { deprecatedDatetime } from "./other";
 
 export enum RuleSeverity {
   ERROR = "ERROR",
   WARNING = "WARNING",
   INFO = "INFO",
+  HINT = "HINT",
 }
 
 export interface LinterRule {
   name: string;
   message: string;
   severity: RuleSeverity;
-  check: (ast: QAst) => Entity[];
+  check: (ast: QAst) => Token[];
 }
 
 const check = () => [];
@@ -60,7 +55,7 @@ const DeclaredAfterUserRule: LinterRule = {
   name: "DECLARED_AFTER_USE",
   message: "The variable was declared after being used",
   severity: RuleSeverity.ERROR,
-  check: declaredAfterUse,
+  check,
 };
 
 const GlobalPeachRule: LinterRule = {
@@ -293,14 +288,14 @@ const UnusedInternalRule: LinterRule = {
 const UnusedParamRule: LinterRule = {
   name: "UNUSED_PARAM",
   message: "This param was declared then never used",
-  severity: RuleSeverity.WARNING,
+  severity: RuleSeverity.HINT,
   check: unusedParam,
 };
 
 const UnusedVarRule: LinterRule = {
   name: "UNUSED_VAR",
   message: "This variable was declared then never used",
-  severity: RuleSeverity.WARNING,
+  severity: RuleSeverity.HINT,
   check: unusedVar,
 };
 
@@ -368,7 +363,7 @@ const LineLengthRule: LinterRule = {
   name: "LINE_LENGTH",
   message: "Maximum line length exceeded",
   severity: RuleSeverity.WARNING,
-  check: lineLength,
+  check,
 };
 
 const DefaultQdocRule: LinterRule = {