feat: Add limited type checking for validate command#5076
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements limited type checking for the validation command, starting with the simplest case of *ast.LiteralExpr expressions. The implementation includes type checking for literal values, arrays, objects, unary expressions, and binary expressions. The PR also handles capsule types correctly, allowing conversions such as discovery.Target to map types.
Key Changes
- Added comprehensive type checking functions in
syntax/typecheck/typecheck.gofor various expression types (literal, array, object, unary, binary) - Refactored binary and unary operation functions from
syntax/vmpackage to a newsyntax/internal/transformpackage for reuse - Enhanced the
valuepackage to exposeTryCapsuleConvertand addEncapsulateWithRvfor better capsule handling
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
syntax/typecheck/typecheck.go |
Added type checking functions for literal, array, object, unary, and binary expressions with proper diagnostic reporting |
syntax/typecheck/typecheck_test.go |
Extended test suite with new test cases covering wrong literal types, arrays, objects, and capsule type checking |
syntax/vm/vm.go |
Refactored to use transform.BinaryOp and transform.UnaryOp instead of internal functions |
syntax/internal/transform/unary.go |
Moved unary operation logic from vm package, renamed function to UnaryOp for public access |
syntax/internal/transform/binary.go |
Moved binary operation logic from vm package, renamed function to BinaryOp for public access |
syntax/internal/value/value.go |
Added EncapsulateWithRv function to create capsule values from reflect.Value |
syntax/internal/value/decode.go |
Made TryCapsuleConvert public and simplified redundant error checking conditions |
internal/validator/testdata/default/*.txtar |
Updated test data files to use appropriate types for proper validation testing |
internal/validator/testdata/default/invalid_types.diags |
Added expected diagnostics for new invalid type test cases |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
syntax/internal/value/value.go:105
- The function documentation incorrectly states "Encapsulate panics" when it should say "EncapsulateWithRv panics" to match the actual function name.
// Encode creates a new Value from v. If v is a pointer, v must be considered
// immutable and not change while the Value is used.
4df3f02 to
70583f5
Compare
1c12de4 to
dde0acd
Compare
dde0acd to
2194d37
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2194d37 to
c67c499
Compare
Contributor
|
TruffleHog Scan Results No secrets detected in this PR. |
x1unix
approved these changes
Feb 18, 2026
Draft
jharvey10
pushed a commit
that referenced
this pull request
Feb 26, 2026
#### PR Description Doing type checking without evaluation is not easy. But I figured we could start with the simplest case `*ast.LiteralExpr`. Because we always know what the values are it's pretty straight forward to add. I also added support for other expressions but it all boils down to performing type checking on Literal expressions. I also try to handle capsules correctly, where e.g. discovery.Target can be converted into a map. #### Which issue(s) this PR fixes Part of: #3712 #### Notes to the Reviewer Next up we can try to type check `*ast.AccessExrp` but for that we need a bit more information passed and I figured I can do that in a follow up pr. #### PR Checklist - [x] CHANGELOG.md updated - [ ] Documentation added - [x] Tests updated - [ ] Config converters updated --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Description
Doing type checking without evaluation is not easy. But I figured we could start with the simplest case
*ast.LiteralExpr.Because we always know what the values are it's pretty straight forward to add. I also added support for other expressions but it all boils down to performing type checking on Literal expressions.
I also try to handle capsules correctly, where e.g. discovery.Target can be converted into a map.
Which issue(s) this PR fixes
Part of: #3712
Notes to the Reviewer
Next up we can try to type check
*ast.AccessExrpbut for that we need a bit more information passed and I figured I can do that in a follow up pr.PR Checklist