Skip to content

fix: prevent panics in case of nils#2971

Merged
miparnisari merged 1 commit intomainfrom
prevent-panics-cause-nil
Mar 18, 2026
Merged

fix: prevent panics in case of nils#2971
miparnisari merged 1 commit intomainfrom
prevent-panics-cause-nil

Conversation

@miparnisari
Copy link
Contributor

Extracting some changes from #2965

@miparnisari miparnisari requested a review from a team as a code owner March 18, 2026 20:32
@github-actions github-actions bot added area/schema Affects the Schema Language area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools) labels Mar 18, 2026
@codecov
Copy link

codecov bot commented Mar 18, 2026

Codecov Report

❌ Patch coverage is 77.35849% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.50%. Comparing base (ea6c5f2) to head (f9b79dd).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pkg/caveats/parameters.go 0.00% 2 Missing and 1 partial ⚠️
pkg/schemadsl/compiler/compiler.go 40.00% 2 Missing and 1 partial ⚠️
pkg/schemadsl/compiler/importer.go 0.00% 2 Missing and 1 partial ⚠️
pkg/schemadsl/compiler/translator.go 88.47% 2 Missing and 1 partial ⚠️

❌ Your project status has failed because the head coverage (73.50%) is below the target coverage (75.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2971      +/-   ##
==========================================
- Coverage   74.83%   73.50%   -1.33%     
==========================================
  Files         498      498              
  Lines       61041    61058      +17     
==========================================
- Hits        45672    44872     -800     
- Misses      12178    12990     +812     
- Partials     3191     3196       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.


// ParameterTypeString returns the string form of the type reference.
func ParameterTypeString(typeRef *core.CaveatTypeReference) string {
if typeRef == nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI this test in the position mapper makes it crash:

{
+                       name: "caveat parameter reference second reference",
+                       schema: `definition user {}
+
+                       caveat somecaveat(someparam int) {
+                               someparam < 42 || someparam > 43
+                       }
+
+                       definition resource {
+                               relation viewer: user with somecaveat
+                               permission view = viewer
+                       }
+                       `,
+                       line:   3,
+                       column: 23,
+                       expectedReference: &SchemaReference{
+                               Source:                   input.Source("test"),
+                               Position:                 input.Position{LineNumber: 3, ColumnPosition: 23},                                                                                              
+                               Text:                     "someparam",
+                               ReferenceType:            ReferenceTypeCaveatParameter,
+                               ReferenceMarkdown:        "someparam int",
+                               TargetSource:             &testSource,
+                               TargetSourceCode:         "someparam int",
+                               TargetNamePositionOffset: 0,
+                       },
+               },

@miparnisari miparnisari force-pushed the prevent-panics-cause-nil branch from fe72a10 to 3900670 Compare March 18, 2026 20:38
hasBlankline: true,
hasNewScope: true,
caveatTypeSet: caveatTypeSet,
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI notice how the flags map isn't set

@miparnisari miparnisari force-pushed the prevent-panics-cause-nil branch from 3900670 to 090e4f5 Compare March 18, 2026 20:50
@miparnisari miparnisari force-pushed the prevent-panics-cause-nil branch from 090e4f5 to f9b79dd Compare March 18, 2026 20:56
@miparnisari miparnisari enabled auto-merge (squash) March 18, 2026 21:00
@miparnisari miparnisari merged commit c4410bc into main Mar 18, 2026
44 of 45 checks passed
@miparnisari miparnisari deleted the prevent-panics-cause-nil branch March 18, 2026 21:20
@github-actions github-actions bot locked and limited conversation to collaborators Mar 18, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area/schema Affects the Schema Language area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants