Skip to content

Commit

Permalink
Add import shadowing rule (#76)
Browse files Browse the repository at this point in the history
Taken from OPA strict mode - we should have it too

Signed-off-by: Anders Eknert <anders@styra.com>
  • Loading branch information
anderseknert authored Apr 11, 2023
1 parent 53b584b commit 780c1dc
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 8 deletions.
5 changes: 5 additions & 0 deletions bundle/regal/ast.rego
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ import future.keywords.every
import future.keywords.if
import future.keywords.in

policy(snippet) := regal.parse_module("policy.rego", concat("", [
"package policy\n\n",
snippet,
]))

# METADATA
# description: parses provided policy with all future keywords imported. Primarily for testing.
with_future_keywords(policy) := regal.parse_module("policy.rego", concat("", [
Expand Down
37 changes: 34 additions & 3 deletions bundle/regal/rules/imports/imports.rego
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,19 @@ import future.keywords.in
import data.regal.config
import data.regal.result

_identifiers := [_ident(imported) |
some imported in input.imports
]

# regular import
_ident(imported) := path[count(path) - 1].value if {
not imported.alias
path := imported.path.value
}

# aliased import
_ident(imported) := imported.alias

# METADATA
# title: implicit-future-keywords
# description: Use explicit future keyword imports
Expand All @@ -29,7 +42,7 @@ report contains violation if {
imported.path.value[1].type == "string"
imported.path.value[1].value == "keywords"

violation := result.fail(rego.metadata.rule(), {"location": imported.path.value[0].location})
violation := result.fail(rego.metadata.rule(), result.location(imported.path.value[0]))
}

# METADATA
Expand All @@ -51,7 +64,7 @@ report contains violation if {
# count(imported.path.value) == 1
# imported.alias

violation := result.fail(rego.metadata.rule(), {"location": imported.path.value[0].location})
violation := result.fail(rego.metadata.rule(), result.location(imported.path.value[0]))
}

# METADATA
Expand All @@ -71,5 +84,23 @@ report contains violation if {

imported.path.value[0].value == "data"

violation := result.fail(rego.metadata.rule(), {"location": imported.path.value[0].location})
violation := result.fail(rego.metadata.rule(), result.location(imported.path.value[0]))
}

# METADATA
# title: import-shadows-import
# description: Import shadows another import
# related_resources:
# - description: documentation
# ref: https://docs.styra.com/regal/rules/import-shadows-import
# custom:
# category: imports
report contains violation if {
config.for_rule(rego.metadata.rule()).enabled == true

some i, identifier in _identifiers

identifier in array.slice(_identifiers, 0, i)

violation := result.fail(rego.metadata.rule(), result.location(input.imports[i].path.value[0]))
}
42 changes: 37 additions & 5 deletions bundle/regal/rules/imports/imports_test.rego
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ test_fail_future_keywords_import_wildcard if {
"ref": "https://docs.styra.com/regal/rules/implicit-future-keywords",
}],
"title": "implicit-future-keywords",
"location": {"col": 8, "file": "policy.rego", "row": 8},
"location": {"col": 8, "file": "policy.rego", "row": 3},
}}
}

Expand All @@ -40,7 +40,7 @@ test_fail_import_input if {
"ref": "https://docs.styra.com/regal/rules/avoid-importing-input",
}],
"title": "avoid-importing-input",
"location": {"col": 8, "file": "policy.rego", "row": 8},
"location": {"col": 8, "file": "policy.rego", "row": 3},
}}
}

Expand All @@ -53,7 +53,7 @@ test_fail_import_data if {
"ref": "https://docs.styra.com/regal/rules/redundant-data-import",
}],
"title": "redundant-data-import",
"location": {"col": 8, "file": "policy.rego", "row": 8},
"location": {"col": 8, "file": "policy.rego", "row": 3},
}}
}

Expand All @@ -66,16 +66,48 @@ test_fail_import_data_aliased if {
"ref": "https://docs.styra.com/regal/rules/redundant-data-import",
}],
"title": "redundant-data-import",
"location": {"col": 8, "file": "policy.rego", "row": 8},
"location": {"col": 8, "file": "policy.rego", "row": 3},
}}
}

test_success_import_data_path if {
report(`import data.something`) == set()
}

test_fail_duplicate_import if {
report(`
import data.foo
import data.foo
`) == {{
"category": "imports",
"description": "Import shadows another import",
"related_resources": [{
"description": "documentation",
"ref": "https://docs.styra.com/regal/rules/import-shadows-import",
}],
"title": "import-shadows-import",
"location": {"col": 9, "file": "policy.rego", "row": 5},
}}
}

test_fail_duplicate_import_alias if {
report(`
import data.foo
import data.bar as foo
`) == {{
"category": "imports",
"description": "Import shadows another import",
"related_resources": [{
"description": "documentation",
"ref": "https://docs.styra.com/regal/rules/import-shadows-import",
}],
"title": "import-shadows-import",
"location": {"col": 9, "file": "policy.rego", "row": 5},
}}
}

report(snippet) := report if {
# regal ignore:input-or-data-reference
report := imports.report with input as ast.with_future_keywords(snippet)
report := imports.report with input as ast.policy(snippet)
with config.for_rule as {"enabled": true}
}

0 comments on commit 780c1dc

Please sign in to comment.