fix: wrong diagnostics for non valid public properties (#877)

* Fix bugg in alg variable diagnostics

* Update pnpm script to new package name

* Add tests to alg visitor

* Fix minor unused code

Author: vetlek

package.json

@@ -28,6 +28,6 @@
     "packages/*"
   ],
   "scripts": {
-    "dev": "pnpm --filter septic watch"
+    "dev": "pnpm --filter septic-config-lib watch"
   }
 }

packages/septic/src/alg.ts

@@ -407,11 +407,8 @@ export interface IAlgVisitor {
 export class AlgVisitor implements IAlgVisitor {
     calcs: AlgCalc[] = [];
     variables: AlgLiteral[] = [];
-    private ignoreIdentifierCalcParams;
 
-    constructor(ignoreIdentifierCalcParams: boolean = false) {
-        this.ignoreIdentifierCalcParams = ignoreIdentifierCalcParams;
-    }
+    constructor() {}
 
     visit(expr: AlgExpr) {
         expr.accept(this);
@@ -435,13 +432,6 @@ export class AlgVisitor implements IAlgVisitor {
     visitCalc(expr: AlgCalc): any {
         this.calcs.push(expr);
         for (const param of expr.params) {
-            if (
-                this.ignoreIdentifierCalcParams &&
-                param instanceof AlgLiteral &&
-                param.type === AlgTokenType.identifier
-            ) {
-                continue;
-            }
             param.accept(this);
         }
     }

packages/septic/src/diagnostics.ts

@@ -278,7 +278,7 @@ export function validateAlg(
         diagnostics.push(diagnostic);
         return diagnostics;
     }
-    const visitor = new AlgVisitor(true);
+    const visitor = new AlgVisitor();
     visitor.visit(expr);
     visitor.calcs.forEach((calc) => {
         diagnostics.push(
@@ -289,9 +289,8 @@ export function validateAlg(
         diagnostics.push(
             ...validateAlgVariable(
                 variable,
-                doc,
                 contextProvider,
-                offsetStartAlg,
+                algPositionTransformer,
             ),
         );
     });
@@ -300,9 +299,8 @@ export function validateAlg(
 
 export function validateAlgVariable(
     variable: AlgLiteral,
-    doc: TextDocument,
     contextProvider: SepticContext,
-    offsetStartAlg: number,
+    algPositionTransformer: AlgPositionTransformer,
 ): SepticDiagnostic[] {
     if (isPureJinja(variable.value)) {
         return [];
@@ -317,10 +315,7 @@ export function validateAlgVariable(
         return [
             createDiagnostic(
                 SepticDiagnosticLevel.warning,
-                {
-                    start: doc.positionAt(offsetStartAlg + variable.start),
-                    end: doc.positionAt(offsetStartAlg + variable.end),
-                },
+                algPositionTransformer(variable.start, variable.end),
                 `Reference to undefined variable: ${variable.value}`,
                 SepticDiagnosticCode.missingReference,
             ),
@@ -333,10 +328,7 @@ export function validateAlgVariable(
         return [
             createDiagnostic(
                 SepticDiagnosticLevel.error,
-                {
-                    start: doc.positionAt(offsetStartAlg + variable.end - 1),
-                    end: doc.positionAt(offsetStartAlg + variable.end),
-                },
+                algPositionTransformer(variable.end - 1, variable.end),
                 `Missing public property for variable`,
                 SepticDiagnosticCode.missingPublicProperty,
             ),
@@ -358,21 +350,15 @@ export function validateAlgVariable(
         return [
             createDiagnostic(
                 SepticDiagnosticLevel.error,
-                {
-                    start: doc.positionAt(
-                        offsetStartAlg +
-                            variable.start +
-                            variableParts[0]!.length +
-                            1,
-                    ),
-                    end: doc.positionAt(offsetStartAlg + variable.end),
-                },
+                algPositionTransformer(
+                    variable.end - variableParts[1]!.length,
+                    variable.end,
+                ),
                 `Unknown public property ${variableParts[1]} for ${referencedObjects[0]!.type}'`,
                 SepticDiagnosticCode.unknownPublicProperty,
             ),
         ];
     }
-
     return [];
 }
 
@@ -593,11 +579,7 @@ function validateParamType(
     algPositionTransformer: AlgPositionTransformer,
 ): SepticDiagnostic[] {
     if (types[0]!.startsWith("value")) {
-        return validateValueParamType(
-            expr,
-            contextProvider,
-            algPositionTransformer,
-        );
+        return [];
     }
     return validateObjectParamType(
         expr,
@@ -642,36 +624,6 @@ function validateObjectParamType(
     ];
 }
 
-function validateValueParamType(
-    expr: AlgExpr,
-    contextProvider: SepticContext,
-    algPositionTransformer: AlgPositionTransformer,
-): SepticDiagnostic[] {
-    if (!isAlgExprObjectReference(expr)) {
-        return [];
-    }
-    const exprLiteral = expr as AlgLiteral;
-    if (isPureJinja(exprLiteral.value)) {
-        return [];
-    }
-    if (
-        contextProvider.validateReferences(
-            exprLiteral.value.split(".")[0]!,
-            defaultRefValidationFunction,
-        )
-    ) {
-        return [];
-    }
-    return [
-        createDiagnostic(
-            SepticDiagnosticLevel.warning,
-            algPositionTransformer(expr.start, expr.end),
-            `Reference to undefined variable: ${exprLiteral.value}`,
-            SepticDiagnosticCode.missingReference,
-        ),
-    ];
-}
-
 function isAlgExprObjectReference(expr: AlgExpr) {
     return expr instanceof AlgLiteral && expr.type === AlgTokenType.identifier;
 }

packages/septic/src/test/algParser.test.ts

@@ -8,6 +8,7 @@ import {
     AlgLiteral,
     parseAlg,
     AlgUnary,
+    AlgVisitor,
 } from "../alg";
 
 describe("Test scanning of operators", () => {
@@ -462,3 +463,30 @@ describe("Test parsing of invalid algs", () => {
         expect(parse).to.not.throw();
     });
 });
+
+describe("Test algVisitor", () => {
+    it("Test extraction of variables in alg", () => {
+        const input =
+            "setmaxdn({{ Wellname }}ZpcNew, -abs({{ Wellname }}ZpcYWCD.Tests)/SPM)";
+        const expr = parseAlg(input);
+        const visitor = new AlgVisitor();
+        visitor.visit(expr);
+        expect(visitor.variables.length).to.equal(3);
+        const variableNames = visitor.variables.map((v) => v.value);
+        expect(variableNames).to.include("{{Wellname}}ZpcNew");
+        expect(variableNames).to.include("{{Wellname}}ZpcYWCD.Tests");
+        expect(variableNames).to.include("SPM");
+    });
+    it("Test extraction of variables in alg", () => {
+        const input =
+            "setmaxdn({{ Wellname }}ZpcNew, -abs({{ Wellname }}ZpcYWCD.Tests)/SPM) + min(10, -10)";
+        const expr = parseAlg(input);
+        const visitor = new AlgVisitor();
+        visitor.visit(expr);
+        expect(visitor.calcs.length).to.equal(3);
+        const calcNames = visitor.calcs.map((v) => v.identifier);
+        expect(calcNames).to.include("setmaxdn");
+        expect(calcNames).to.include("abs");
+        expect(calcNames).to.include("min");
+    });
+});

packages/septic/src/test/diagnostics.test.ts

@@ -197,6 +197,21 @@ describe("Test diagnostics for references in algs", () => {
             SepticDiagnosticCode.unknownPublicProperty,
         );
     });
+    it("Expect diagnostics for unknown property when variable is first param in calc", () => {
+        const text = `
+            Mvr: TestMvr
+			CalcPvr:  TestCalcPvr 
+				Text1= "Test"
+				Alg= "-abs(TestMvr.Tests)"
+		`;
+
+        const cnfg = parseSepticForTest(text);
+        const diag = validateAlgs(cnfg, cnfg);
+        expect(diag.length).to.equal(1);
+        expect(diag[0]!.code).to.equal(
+            SepticDiagnosticCode.unknownPublicProperty,
+        );
+    });
 });
 
 describe("Test datatype diagnostics in algs", () => {