Skip to content

TPE: Use helper functions to build the return value#2301

Merged
john-h-kastner-aws merged 1 commit intocedar-policy:mainfrom
luxas:tpe-return-helpers
Apr 13, 2026
Merged

TPE: Use helper functions to build the return value#2301
john-h-kastner-aws merged 1 commit intocedar-policy:mainfrom
luxas:tpe-return-helpers

Conversation

@luxas
Copy link
Copy Markdown
Member

@luxas luxas commented Apr 8, 2026

Description of changes

Broken out from #2162, prefactoring.
This PR makes it clear that the type information is not actually used to make any decisions (yet) during TPE (ref: #2092).
This should decrease a bit of the cognitive complexity when reading the source code too, hopefully.
In #2162, it will become convenient to add logic in these helper functions, e.g. preserving the Loc of the residual, or adding a Loc to the error type.

Issue #, if available

Checklist for requesting a review

The change in this PR is (choose one, and delete the other options):

  • A change "invisible" to users (e.g., documentation, changes to "internal" crates like cedar-policy-core, cedar-validator, etc.)

I confirm that this PR (choose one, and delete the other options):

  • Does not update the CHANGELOG because my change does not significantly impact released code.

I confirm that cedar-spec (choose one, and delete the other options):

  • Does not require updates because my change does not impact the Cedar formal model or DRT infrastructure.

I confirm that docs.cedarpolicy.com (choose one, and delete the other options):

  • Does not require updates because my change does not impact the Cedar language specification.

Signed-off-by: Lucas Käldström <lucas.kaldstrom@upbound.io>
@luxas luxas force-pushed the tpe-return-helpers branch from 56a0f5a to 41ecf77 Compare April 8, 2026 15:37
value: principal.into(),
ty,
}
mk_concrete(principal.into())
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the VS Code diff is nicer in that it shows that principal.into() stayed intact here, but Github does not, even in hide whitespace mode

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2026

Coverage Report

Head Commit: 41ecf77d7ef77626d983eea9b61268630caeea8d

Base Commit: 16bb13698aaffc4e6dd08458584516456d3dfebb

Download the full coverage report.

Coverage of Added or Modified Lines of Rust Code

Required coverage: 80.00%

Actual coverage: 79.61%

Status: FAILED ❌

Details
File Status Covered Coverage Missed Lines
cedar-policy-core/src/tpe/evaluator.rs 🟡 121/152 79.61% 78, 123, 182, 243, 263, 276, 290, 295, 319, 335, 357, 365, 368, 371, 382, 385, 388, 391, 394, 410, 413, 418, 424, 430, 434, 455, 479, 504, 509, 541, 546

Coverage of All Lines of Rust Code

Required coverage: 80.00%

Actual coverage: 87.60%

Status: PASSED ✅

Details
Package Status Covered Coverage Base Coverage
cedar-language-server 🟢 4722/5102 92.55% --
cedar-policy 🟡 4227/5355 78.94% --
cedar-policy-cli 🟡 1210/1614 74.97% --
cedar-policy-core 🟢 23821/27090 87.93% --
cedar-policy-formatter 🟢 914/1088 84.01% --
cedar-policy-symcc 🟢 6752/7262 92.98% --
cedar-wasm 🔴 0/28 0.00% --

@john-h-kastner-aws
Copy link
Copy Markdown
Contributor

You've obviously not written any new uncovered code here. But, if you're in the mood, I think some of the uncovered lines being reported should be easy enough to reach with a test case. (some others are type errors which we don't expect to be reachable)

Copy link
Copy Markdown
Contributor

@john-h-kastner-aws john-h-kastner-aws left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great

@john-h-kastner-aws john-h-kastner-aws merged commit a51dbb9 into cedar-policy:main Apr 13, 2026
23 of 24 checks passed
@luxas
Copy link
Copy Markdown
Member Author

luxas commented Apr 14, 2026

You've obviously not written any new uncovered code here. But, if you're in the mood, I think some of the uncovered lines being reported should be easy enough to reach with a test case. (some others are type errors which we don't expect to be reachable)

Yeah, I'll look at this in some of the coming PRs to see if I can make the coverage checker happier 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants