From 1c4143e40edaeb5360e8058acb1bd889aa5d0ccd Mon Sep 17 00:00:00 2001 From: Gusarich Date: Fri, 14 Jun 2024 12:38:36 +0300 Subject: [PATCH] resolve review comments --- .../contracts/underscore-variable.tact | 2 + .../e2e-emulated/underscore-variable.spec.ts | 2 +- .../resolveStatements.spec.ts.snap | 10 ++- src/types/resolveExpression.ts | 6 ++ src/types/resolveStatements.ts | 65 ++++++++----------- .../stmts/var-underscore-name-in-let.tact | 1 + 6 files changed, 44 insertions(+), 42 deletions(-) diff --git a/src/test/e2e-emulated/contracts/underscore-variable.tact b/src/test/e2e-emulated/contracts/underscore-variable.tact index 41c11e091..73fbaeef7 100644 --- a/src/test/e2e-emulated/contracts/underscore-variable.tact +++ b/src/test/e2e-emulated/contracts/underscore-variable.tact @@ -50,6 +50,8 @@ contract UnderscoreVariableTestContract { get fun test4(): Int { let _: Int = self.increaseSomething(); let _: Int = self.increaseSomething(); + let _ = self.increaseSomething(); + let _ = self.increaseSomething(); return self.something; } } \ No newline at end of file diff --git a/src/test/e2e-emulated/underscore-variable.spec.ts b/src/test/e2e-emulated/underscore-variable.spec.ts index cb08eb185..e447c2cda 100644 --- a/src/test/e2e-emulated/underscore-variable.spec.ts +++ b/src/test/e2e-emulated/underscore-variable.spec.ts @@ -21,6 +21,6 @@ describe("underscore-variable", () => { expect(await contract.getTest1()).toEqual(0n); expect(await contract.getTest2()).toEqual(12n); expect(await contract.getTest3()).toEqual(6n); - expect(await contract.getTest4()).toEqual(2n); + expect(await contract.getTest4()).toEqual(4n); }); }); diff --git a/src/types/__snapshots__/resolveStatements.spec.ts.snap b/src/types/__snapshots__/resolveStatements.spec.ts.snap index 7314a2bec..c8d73e1c3 100644 --- a/src/types/__snapshots__/resolveStatements.spec.ts.snap +++ b/src/types/__snapshots__/resolveStatements.spec.ts.snap @@ -581,7 +581,7 @@ Line 7, col 9: `; exports[`resolveStatements should fail statements for var-underscore-name-access 1`] = ` -":6:16: Unable to resolve id _ +":6:16: Wildcard variable name '_' cannot be accessed Line 6, col 16: 5 | foreach (_, _ in m) { > 6 | return _; @@ -591,7 +591,7 @@ Line 6, col 16: `; exports[`resolveStatements should fail statements for var-underscore-name-access2 1`] = ` -":7:14: Unable to resolve id _ +":7:14: Wildcard variable name '_' cannot be accessed Line 7, col 14: 6 | foreach (_, v in m) { > 7 | x += _; @@ -601,7 +601,7 @@ Line 7, col 14: `; exports[`resolveStatements should fail statements for var-underscore-name-access3 1`] = ` -":9:12: Unable to resolve id _ +":9:12: Wildcard variable name '_' cannot be accessed Line 9, col 12: 8 | let _: Int = someImpureFunction(); > 9 | return _; @@ -1685,6 +1685,10 @@ exports[`resolveStatements should resolve statements for var-underscore-name-in- "someImpureFunction()", "Int", ], + [ + "someImpureFunction()", + "Int", + ], [ "123", "Int", diff --git a/src/types/resolveExpression.ts b/src/types/resolveExpression.ts index 99d72ac27..33c920846 100644 --- a/src/types/resolveExpression.ts +++ b/src/types/resolveExpression.ts @@ -766,6 +766,12 @@ export function resolveExpression( const v = sctx.vars.get(exp.value); if (!v) { if (!hasStaticConstant(ctx, exp.value)) { + if (exp.value === "_") { + throwError( + "Wildcard variable name '_' cannot be accessed", + exp.ref, + ); + } throwError("Unable to resolve id " + exp.value, exp.ref); } else { const cc = getStaticConstant(ctx, exp.value); diff --git a/src/types/resolveStatements.ts b/src/types/resolveStatements.ts index 1e1cbd574..3f39c9898 100644 --- a/src/types/resolveStatements.ts +++ b/src/types/resolveStatements.ts @@ -29,6 +29,20 @@ function emptyContext(root: ASTRef, returns: TypeRef): StatementContext { }; } +function checkVariableExists( + ctx: StatementContext, + name: string, + ref?: ASTRef, +): void { + if (ctx.vars.has(name)) { + if (ref) { + throwError(`Variable already exists: "${name}"`, ref); + } else { + throw Error(`Variable already exists: "${name}"`); + } + } +} + function addRequiredVariables( name: string, src: StatementContext, @@ -61,11 +75,9 @@ function addVariable( ref: TypeRef, src: StatementContext, ): StatementContext { - if (src.vars.has(name)) { - throw Error("Variable already exists: " + name); // Should happen earlier - } + checkVariableExists(src, name); // Should happen earlier if (name == "_") { - throw Error("Variable name cannot be '_'"); + return src; } return { ...src, @@ -174,9 +186,7 @@ function processStatements( ctx = resolveExpression(s.expression, sctx, ctx); // Check variable name - if (sctx.vars.has(s.name)) { - throwError(`Variable "${s.name}" already exists`, s.ref); - } + checkVariableExists(sctx, s.name, s.ref); // Check type const expressionType = getExpType(ctx, s.expression); @@ -188,16 +198,12 @@ function processStatements( s.ref, ); } - if (s.name !== "_") { - sctx = addVariable(s.name, variableType, sctx); - } + sctx = addVariable(s.name, variableType, sctx); } else { if (expressionType.kind === "null") { throwError(`Cannot infer type for "${s.name}"`, s.ref); } - if (s.name !== "_") { - sctx = addVariable(s.name, expressionType, sctx); - } + sctx = addVariable(s.name, expressionType, sctx); } } else if (s.kind === "statement_assign") { // Process lvalue @@ -398,19 +404,12 @@ function processStatements( let catchCtx = sctx; // Process catchName variable for exit code - if (s.catchName != "_") { - if (initialCtx.vars.has(s.catchName)) { - throwError( - `Variable already exists: "${s.catchName}"`, - s.ref, - ); - } - catchCtx = addVariable( - s.catchName, - { kind: "ref", name: "Int", optional: false }, - initialCtx, - ); - } + checkVariableExists(initialCtx, s.catchName, s.ref); + catchCtx = addVariable( + s.catchName, + { kind: "ref", name: "Int", optional: false }, + initialCtx, + ); // Process catch statements const rCatch = processStatements(s.catchStatements, catchCtx, ctx); @@ -450,12 +449,7 @@ function processStatements( // Add key and value to statement context if (s.keyName != "_") { - if (initialCtx.vars.has(s.keyName)) { - throwError( - `Variable already exists: "${s.keyName}"`, - s.ref, - ); - } + checkVariableExists(initialCtx, s.keyName, s.ref); foreachCtx = addVariable( s.keyName, { kind: "ref", name: mapType.key, optional: false }, @@ -463,12 +457,7 @@ function processStatements( ); } if (s.valueName != "_") { - if (foreachCtx.vars.has(s.valueName)) { - throwError( - `Variable already exists: "${s.valueName}"`, - s.ref, - ); - } + checkVariableExists(foreachCtx, s.valueName, s.ref); foreachCtx = addVariable( s.valueName, { kind: "ref", name: mapType.value, optional: false }, diff --git a/src/types/stmts/var-underscore-name-in-let.tact b/src/types/stmts/var-underscore-name-in-let.tact index 9c2ac2d9b..6422a59bc 100644 --- a/src/types/stmts/var-underscore-name-in-let.tact +++ b/src/types/stmts/var-underscore-name-in-let.tact @@ -6,5 +6,6 @@ fun someImpureFunction(): Int { fun test(): Int { let _: Int = someImpureFunction(); + let _ = someImpureFunction(); return 123; }