-
Notifications
You must be signed in to change notification settings - Fork 13
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
✨ lint error when query is used both as check and data query #1380
Conversation
Test Results 1 files 24 suites 20s ⏱️ Results for commit f8db0d1. ♻️ This comment has been updated with latest results. |
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Ivan Milchev <[email protected]>
e1ae4ec
to
28610de
Compare
internal/bundle/lint.go
Outdated
@@ -414,6 +417,20 @@ func lintFile(file string) (*Results, error) { | |||
uid := check.Uid | |||
updateAssignedQueries(check, assignedQueries, globalQueriesByUid) | |||
|
|||
checks[check.CodeId] = struct{}{} | |||
if _, ok := dataQueries[check.CodeId]; ok { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at this point in the code, CodeId
isn't set. If we move this logic to outside of this function, the file context is missing...
If we want to implement this error, we will need to use this in combination with the ocmpiled bundle map. The issue is that the bundlemap indexes queries by MRN, but this struct has only UIDs. There is no easy way to match which query gets which mrn/codeid
4cdf274
to
72ac6a7
Compare
@@ -414,6 +417,20 @@ func lintFile(file string) (*Results, error) { | |||
uid := check.Uid | |||
updateAssignedQueries(check, assignedQueries, globalQueriesByUid) | |||
|
|||
checks[check.Uid] = struct{}{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we only check by uid since this is under user control. It is still possible to cause weirdness when the MQL is identical for a query and a check but that's a bug in the resolved policy. It's not something to be caught by the linter
Signed-off-by: Ivan Milchev <[email protected]>
72ac6a7
to
3bb0649
Compare
@@ -34,6 +34,7 @@ const ( | |||
queryTitle = "query-name" | |||
queryUidUnique = "query-uid-unique" | |||
queryUnassigned = "query-unassigned" | |||
queryUsedAsDifferentTypes = "query-used-as-different-types" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think you need to update the LinterRules
var with this entry too so that it shows in the sarif report
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point! I missed that
Signed-off-by: Ivan Milchev <[email protected]>
No description provided.