Skip to content
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

Fix detached-metadata issues #1143

Merged
merged 1 commit into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions bundle/regal/rules/style/detached-metadata/detached_metadata.rego
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import data.regal.result
import data.regal.util

report contains violation if {
some block in ast.comments.blocks
some i, block in ast.comments.blocks

startswith(trim_space(block[0].text), "METADATA")

Expand All @@ -18,8 +18,7 @@ report contains violation if {
# no need to +1 the index here as rows start counting from 1
trim_space(input.regal.file.lines[last_row]) == ""

annotation := _annotation_at_row(util.to_location_object(block[0].location).row)
annotation.scope != "document"
not _allow_detached(last_row, i, ast.comments.blocks, input.regal.file.lines)

violation := result.fail(rego.metadata.chain(), result.location(block[0]))
}
Expand All @@ -29,3 +28,18 @@ _annotation_at_row(row) := annotation if {

util.to_location_object(annotation.location).row == row
}

# detached metadata is allowed only if another metadata block follows
# directly after the metadata block
_allow_detached(last_row, i, blocks, lines) if {
Copy link
Member

Choose a reason for hiding this comment

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

I understand this now, but a comment might be helpful in a few months time! 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure! :) I'll add one 👍

next_block := blocks[i + 1]

startswith(trim_space(next_block[0].text), "METADATA")

next_block_row := util.to_location_object(next_block[0].location).row
lines_between := array.slice(lines, last_row, next_block_row - 1)

every line in lines_between {
line == ""
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ allow := true
r == set()
}

test_success_detached_document_scope_ok if {
test_success_detached_but_more_metadata_on_rule if {
r := rule.report with input as ast.with_rego_v1(`
# METADATA
# scope: document
Expand All @@ -74,6 +74,44 @@ allow := true
r == set()
}

test_success_detached_but_more_metadata_on_package if {
r := rule.report with input as regal.parse_module("p.rego", `
# METADATA
# scope: package
# description: foo

# METADATA
# title: allow
# scope: subpackages
package p
`)
r == set()
}

test_fail_second_block_detached_first_not_reported if {
r := rule.report with input as regal.parse_module("p.rego", `# METADATA
# scope: package
# description: foo

# METADATA
# title: allow
# scope: subpackages

package p
`)
r == {{
"category": "style",
"description": "Detached metadata annotation",
"level": "error",
"location": {"col": 1, "file": "p.rego", "row": 5, "text": "# METADATA"},
"related_resources": [{
"description": "documentation",
"ref": config.docs.resolve_url("$baseUrl/$category/detached-metadata", "style"),
}],
"title": "detached-metadata",
}}
}

test_success_not_detached_by_comment_in_different_column if {
r := rule.report with input as ast.with_rego_v1(`
# METADATA
Expand Down