-
Notifications
You must be signed in to change notification settings - Fork 6
Add type checking for ident expressions #279
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
b4d6e9b
a02360b
1734233
db83f96
35aafba
eae7763
6c0c65a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -17,18 +17,23 @@ import type { | |||||||||
| Constant, | ||||||||||
| Expr, | ||||||||||
| SourceInfo, | ||||||||||
| Expr_Ident, | ||||||||||
| ConstantSchema, | ||||||||||
| } from "@bufbuild/cel-spec/cel/expr/syntax_pb.js"; | ||||||||||
| import { | ||||||||||
| type CheckedExpr, | ||||||||||
| CheckedExprSchema, | ||||||||||
| type ReferenceSchema, | ||||||||||
| type Type, | ||||||||||
| type TypeSchema, | ||||||||||
| Type_PrimitiveType, | ||||||||||
| } from "@bufbuild/cel-spec/cel/expr/checked_pb.js"; | ||||||||||
| import { create, type MessageInitShape } from "@bufbuild/protobuf"; | ||||||||||
| import { | ||||||||||
| CelScalar, | ||||||||||
| celType, | ||||||||||
| type CelType, | ||||||||||
| type CelValue, | ||||||||||
| DURATION, | ||||||||||
| listType, | ||||||||||
| type mapKeyType, | ||||||||||
|
|
@@ -37,17 +42,24 @@ import { | |||||||||
| TIMESTAMP, | ||||||||||
| } from "./type.js"; | ||||||||||
| import { NullValue } from "@bufbuild/protobuf/wkt"; | ||||||||||
| import type { CelEnv } from "./env.js"; | ||||||||||
| import { resolveCandidateNames } from "./namespace.js"; | ||||||||||
| import { celError } from "./error.js"; | ||||||||||
| import { identReference, type ReferenceInfo } from "./referenceinfo.js"; | ||||||||||
|
|
||||||||||
| export class Checker { | ||||||||||
| private readonly referenceMap: Map<bigint, ReferenceInfo> = new Map(); | ||||||||||
| private readonly typeMap: Map<bigint, CelType> = new Map(); | ||||||||||
|
|
||||||||||
| constructor(private readonly env: CelEnv) {} | ||||||||||
|
|
||||||||||
| check(expr: Expr, sourceInfo: SourceInfo | undefined): CheckedExpr { | ||||||||||
| // Clear each time we check since Checker instances are cached per environment. | ||||||||||
| this.typeMap.clear(); | ||||||||||
| return create(CheckedExprSchema, { | ||||||||||
| expr: this.checkExpr(expr), | ||||||||||
| sourceInfo, | ||||||||||
| // TODO: referenceMap | ||||||||||
| referenceMap: celReferenceMapToProtoReferenceMap(this.referenceMap), | ||||||||||
| typeMap: celTypeMapToProtoTypeMap(this.typeMap), | ||||||||||
| }); | ||||||||||
| } | ||||||||||
|
|
@@ -56,6 +68,8 @@ export class Checker { | |||||||||
| switch (expr.exprKind.case) { | ||||||||||
| case "constExpr": | ||||||||||
| return this.checkConstExpr(expr.id, expr.exprKind.value); | ||||||||||
| case "identExpr": | ||||||||||
| return this.checkIdentExpr(expr.id, expr.exprKind.value); | ||||||||||
| default: | ||||||||||
| throw new Error(`Unsupported expression kind: ${expr.exprKind.case}`); | ||||||||||
| } | ||||||||||
|
|
@@ -107,9 +121,49 @@ export class Checker { | |||||||||
| }; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| private checkIdentExpr( | ||||||||||
| id: bigint, | ||||||||||
| ident: Expr_Ident, | ||||||||||
| ): MessageInitShape<typeof ExprSchema> { | ||||||||||
| const found = this.resolveVariable(ident.name); | ||||||||||
| if (found) { | ||||||||||
| this.setType(id, found); | ||||||||||
| this.setReference(id, identReference(ident.name)); | ||||||||||
| return { | ||||||||||
| id, | ||||||||||
| exprKind: { | ||||||||||
| case: "identExpr", | ||||||||||
| value: ident, | ||||||||||
| }, | ||||||||||
| }; | ||||||||||
| } | ||||||||||
| throw celError( | ||||||||||
| `undeclared reference to '${ident.name}' (in container '${this.env.namespace}')`, | ||||||||||
| id, | ||||||||||
| ); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| private setType(id: bigint, type: CelType): void { | ||||||||||
| this.typeMap.set(id, type); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| private setReference(id: bigint, reference: ReferenceInfo): void { | ||||||||||
| this.referenceMap.set(id, reference); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| private resolveVariable(name: string): CelType | undefined { | ||||||||||
| const ident = this.env.variables.find(name); | ||||||||||
| if (ident) { | ||||||||||
| return ident; | ||||||||||
| } | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should always resolve candidate names because there is a well defined order for them
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think when we get to macros it will be an issue. The macro vars can shadow global namespaced vars
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Revisiting the go code this is based on, they make it clearer that this early return should be a local variable and explicitly not a global. I've updated this method and VariableScope to make the different flavors of variable resolution more explicit. |
||||||||||
| for (const candidate of resolveCandidateNames(this.env.namespace, name)) { | ||||||||||
| const ident = this.env.variables.find(candidate); | ||||||||||
| if (ident) { | ||||||||||
| return ident; | ||||||||||
| } | ||||||||||
| } | ||||||||||
| return undefined; | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| export function protoTypeToCelType(pt: Type): CelType { | ||||||||||
|
|
@@ -272,3 +326,72 @@ function celTypeMapToProtoTypeMap( | |||||||||
| } | ||||||||||
| return protoTypeMap; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| function protoConstant< | ||||||||||
| T extends Exclude<Constant["constantKind"]["case"], undefined>, | ||||||||||
| >( | ||||||||||
| caseName: T, | ||||||||||
| value: Extract<Constant["constantKind"], { case: T }>["value"], | ||||||||||
| ): MessageInitShape<typeof ConstantSchema> { | ||||||||||
| return { constantKind: { case: caseName, value } } as MessageInitShape< | ||||||||||
| typeof ConstantSchema | ||||||||||
| >; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| function celValueToProtoConstant( | ||||||||||
| value: CelValue, | ||||||||||
| ): MessageInitShape<typeof ConstantSchema> { | ||||||||||
| const type = celType(value); | ||||||||||
| switch (type.kind) { | ||||||||||
| case "scalar": | ||||||||||
| switch (type.name) { | ||||||||||
| case "bool": | ||||||||||
| return protoConstant("boolValue", value as boolean); | ||||||||||
| case "bytes": | ||||||||||
| return protoConstant("bytesValue", value as Uint8Array); | ||||||||||
| case "double": | ||||||||||
| return protoConstant("doubleValue", value as number); | ||||||||||
| case "int": | ||||||||||
| return protoConstant("int64Value", BigInt(value as number | bigint)); | ||||||||||
| case "string": | ||||||||||
| return protoConstant("stringValue", value as string); | ||||||||||
| case "uint": | ||||||||||
| return protoConstant("uint64Value", BigInt(value as number | bigint)); | ||||||||||
| case "null_type": | ||||||||||
| return protoConstant("nullValue", NullValue.NULL_VALUE); | ||||||||||
| case "dyn": | ||||||||||
| throw new Error("cannot convert dyn to proto constant"); | ||||||||||
| case "type": | ||||||||||
| throw new Error("cannot convert type to proto constant"); | ||||||||||
| } | ||||||||||
| default: | ||||||||||
| throw new Error(`unsupported constant type: ${type.toString()}`); | ||||||||||
| } | ||||||||||
|
srikrsna-buf marked this conversation as resolved.
Outdated
|
||||||||||
| } | ||||||||||
|
|
||||||||||
| function celReferenceMapToProtoReferenceMap( | ||||||||||
| referenceMap: Map<bigint, ReferenceInfo>, | ||||||||||
| ): Record<string, MessageInitShape<typeof ReferenceSchema>> { | ||||||||||
| const protoReferenceMap: Record< | ||||||||||
| string, | ||||||||||
| MessageInitShape<typeof ReferenceSchema> | ||||||||||
| > = {}; | ||||||||||
| for (const [exprId, referenceInfo] of referenceMap.entries()) { | ||||||||||
| if (referenceInfo.overloadIds.size > 0) { | ||||||||||
| protoReferenceMap[exprId.toString()] = { | ||||||||||
| name: referenceInfo.name, | ||||||||||
| overloadId: Array.from(referenceInfo.overloadIds), | ||||||||||
| value: undefined, | ||||||||||
| }; | ||||||||||
| continue; | ||||||||||
| } | ||||||||||
| protoReferenceMap[exprId.toString()] = { | ||||||||||
| name: referenceInfo.name, | ||||||||||
| overloadId: [], | ||||||||||
| value: referenceInfo.value | ||||||||||
| ? celValueToProtoConstant(referenceInfo.value as CelValue) | ||||||||||
| : undefined, | ||||||||||
| }; | ||||||||||
| } | ||||||||||
| return protoReferenceMap; | ||||||||||
| } | ||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| // Copyright 2024-2026 Buf Technologies, Inc. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| import * as assert from "node:assert/strict"; | ||
| import { suite, test } from "node:test"; | ||
| import { ADD_BYTES, ADD_DOUBLE } from "./gen/dev/cel/expr/overload_const.js"; | ||
| import { | ||
| addOverloadIds, | ||
| functionReference, | ||
| identReference, | ||
| } from "./referenceinfo.js"; | ||
| import { toCel } from "./value.js"; | ||
|
|
||
| void suite("ReferenceInfo", () => { | ||
| void test("equals", () => { | ||
| const testCases = [ | ||
| { | ||
| name: "single overload equal", | ||
| a: functionReference([ADD_BYTES]), | ||
| b: functionReference([ADD_BYTES]), | ||
| equal: true, | ||
| }, | ||
| { | ||
| name: "single overload not equal", | ||
| a: functionReference([ADD_BYTES]), | ||
| b: functionReference([ADD_DOUBLE]), | ||
| equal: false, | ||
| }, | ||
| { | ||
| name: "single and multiple overload not equal", | ||
| a: functionReference([ADD_BYTES]), | ||
| b: functionReference([ADD_BYTES, ADD_DOUBLE]), | ||
| equal: false, | ||
| }, | ||
| { | ||
| name: "multiple overloads equal", | ||
| a: functionReference([ADD_BYTES, ADD_DOUBLE]), | ||
| b: functionReference([ADD_DOUBLE, ADD_BYTES]), | ||
| equal: true, | ||
| }, | ||
| { | ||
| name: "identifier reference equal", | ||
| a: identReference("BYTES"), | ||
| b: identReference("BYTES"), | ||
| equal: true, | ||
| }, | ||
| { | ||
| name: "identifier reference not equal", | ||
| a: identReference("BYTES"), | ||
| b: identReference("TRUE"), | ||
| equal: false, | ||
| }, | ||
| { | ||
| name: "identifier and constant reference not equal", | ||
| a: identReference("BYTES"), | ||
| b: identReference("BYTES", toCel(new TextEncoder().encode("bytes"))), | ||
| equal: false, | ||
| }, | ||
| { | ||
| name: "constant references equal", | ||
| a: identReference("BYTES", toCel(new TextEncoder().encode("bytes"))), | ||
| b: identReference("BYTES", toCel(new TextEncoder().encode("bytes"))), | ||
| equal: true, | ||
| }, | ||
| { | ||
| name: "constant references not equal", | ||
| a: identReference("BYTES", toCel(new TextEncoder().encode("bytes"))), | ||
| b: identReference( | ||
| "BYTES", | ||
| toCel(new TextEncoder().encode("bytes-other")), | ||
| ), | ||
| equal: false, | ||
| }, | ||
| { | ||
| name: "constant and overload reference not equal", | ||
| a: identReference("BYTES", toCel(new TextEncoder().encode("bytes"))), | ||
| b: functionReference([ADD_DOUBLE, ADD_BYTES]), | ||
| equal: false, | ||
| }, | ||
| ]; | ||
| for (const tc of testCases) { | ||
| assert.equal( | ||
| tc.a.equals(tc.b), | ||
| tc.equal, | ||
| `unexpected equality for ${tc.name}`, | ||
| ); | ||
| } | ||
| }); | ||
|
|
||
| void test("add overload", () => { | ||
| let add = functionReference([ADD_BYTES]); | ||
| add = addOverloadIds(add, ADD_DOUBLE); | ||
| assert.equal(functionReference([ADD_BYTES, ADD_DOUBLE]).equals(add), true); | ||
| add = addOverloadIds(add, ADD_DOUBLE); | ||
| assert.equal(functionReference([ADD_BYTES, ADD_DOUBLE]).equals(add), true); | ||
| }); | ||
| }); |
Uh oh!
There was an error while loading. Please reload this page.