Rule: directory-package-mismatch

README.md

@@ -210,6 +210,7 @@ The following rules are currently available:
 | idiomatic   | [boolean-assignment](https://docs.styra.com/regal/rules/idiomatic/boolean-assignment)                 | Prefer `if` over boolean assignment                       |
 | idiomatic   | [custom-has-key-construct](https://docs.styra.com/regal/rules/idiomatic/custom-has-key-construct)     | Custom function may be replaced by `in` and `object.keys` |
 | idiomatic   | [custom-in-construct](https://docs.styra.com/regal/rules/idiomatic/custom-in-construct)               | Custom function may be replaced by `in` keyword           |
+| idiomatic   | [directory-package-mismatch](https://docs.styra.com/regal/rules/idiomatic/directory-package-mismatch) | Directory structure should mirror package                 |
 | idiomatic   | [equals-pattern-matching](https://docs.styra.com/regal/rules/idiomatic/equals-pattern-matching)       | Prefer pattern matching in function arguments             |
 | idiomatic   | [no-defined-entrypoint](https://docs.styra.com/regal/rules/idiomatic/no-defined-entrypoint)           | Missing entrypoint annotation                             |
 | idiomatic   | [non-raw-regex-pattern](https://docs.styra.com/regal/rules/idiomatic/non-raw-regex-pattern)           | Use raw strings for regex patterns                        |

build/workflows/update-example-index/process.rego

@@ -1,3 +1,4 @@
+# regal ignore:directory-package-mismatch
 package process
 
 import rego.v1

bundle/regal/config/provided/data.yaml

@@ -69,6 +69,9 @@ rules:
       level: error
     custom-in-construct:
       level: error
+    directory-package-mismatch:
+      level: error
+      exclude-test-suffix: true
     equals-pattern-matching:
       level: error
     no-defined-entrypoint:

bundle/regal/lsp/completion/location/location_test.rego

@@ -1,4 +1,4 @@
-package regal.lsp.completion.providers.location_test
+package regal.lsp.completion.location_test
 
 import rego.v1
 

bundle/regal/lsp/completion/providers/commonrule/commonrule_test.rego

@@ -3,7 +3,7 @@ package regal.lsp.completion.providers.commonrule_test
 import rego.v1
 
 import data.regal.lsp.completion.providers.commonrule as provider
-import data.regal.lsp.completion.providers.utils_test as util
+import data.regal.lsp.completion.providers.test_utils as util
 
 test_common_name_completion_on_invoked if {
 	policy := `package policy

bundle/regal/lsp/completion/providers/default/default_test.rego

@@ -3,7 +3,7 @@ package regal.lsp.completion.providers.default_test
 import rego.v1
 
 import data.regal.lsp.completion.providers["default"] as provider
-import data.regal.lsp.completion.providers.utils_test as util
+import data.regal.lsp.completion.providers.test_utils as util
 
 test_default_completion_on_typing if {
 	policy := `package policy

bundle/regal/lsp/completion/providers/import/import_test.rego

@@ -3,7 +3,7 @@ package regal.lsp.completion.providers.import_test
 import rego.v1
 
 import data.regal.lsp.completion.providers["import"] as provider
-import data.regal.lsp.completion.providers.utils_test as util
+import data.regal.lsp.completion.providers.test_utils as util
 
 test_import_completion_empty_line if {
 	policy := `package policy

bundle/regal/lsp/completion/providers/locals/locals_test.rego

@@ -3,7 +3,7 @@ package regal.lsp.completion.providers.locals_test
 import rego.v1
 
 import data.regal.lsp.completion.providers.locals as provider
-import data.regal.lsp.completion.providers.utils_test as utils
+import data.regal.lsp.completion.providers.test_utils as utils
 
 test_no_locals_in_completion_items if {
 	workspace := {"file:///p.rego": `package policy

bundle/regal/lsp/completion/providers/package/package_test.rego

@@ -3,7 +3,7 @@ package regal.lsp.completion.providers.package_test
 import rego.v1
 
 import data.regal.lsp.completion.providers["package"] as provider
-import data.regal.lsp.completion.providers.utils_test as util
+import data.regal.lsp.completion.providers.test_utils as util
 
 test_package_completion_on_typing if {
 	policy := `p`

bundle/regal/lsp/completion/providers/regov1/regov1_test.rego

@@ -2,7 +2,7 @@ package regal.lsp.completion.providers.regov1_test
 
 import rego.v1
 
-import data.regal.lsp.completion.providers.utils_test as util
+import data.regal.lsp.completion.providers.test_utils as util
 
 import data.regal.lsp.completion.providers.regov1 as provider
 

bundle/regal/lsp/completion/providers/snippet/snippet_test.rego

@@ -3,7 +3,7 @@ package regal.lsp.completion.providers.snippet_test
 import rego.v1
 
 import data.regal.lsp.completion.providers.snippet as provider
-import data.regal.lsp.completion.providers.utils_test as util
+import data.regal.lsp.completion.providers.test_utils as util
 
 # regal ignore:rule-length
 test_snippet_completion_on_typing_partial_prefix if {

bundle/regal/lsp/completion/providers/utils_test.rego → bundle/regal/lsp/completion/providers/test_utils/test_utils.rego

@@ -1,4 +1,4 @@
-package regal.lsp.completion.providers.utils_test
+package regal.lsp.completion.providers.test_utils
 
 import rego.v1
 

internal/lsp/completions/refs/rego/ref_names.rego → bundle/regal/lsp/completion/ref_names.rego

@@ -1,4 +1,4 @@
-package lsp.completions
+package regal.lsp.completion
 
 import rego.v1
 

bundle/regal/rules/idiomatic/directory-package-mismatch/directory_package_mismatch.rego

@@ -0,0 +1,83 @@
+# METADATA
+# description: Directory structure should mirror package
+package regal.rules.idiomatic["directory-package-mismatch"]
+
+import rego.v1
+
+import data.regal.config
+import data.regal.result
+
+# METADATA
+# description: |
+#   emit warning notice when package has more parts than the directory,
+#   as this should likely **not** fail
+notices contains _notice(message, "warning") if {
+	count(_file_path_values) > 0
+	count(_file_path_values) < count(_pkg_path_values)
+
+	message := sprintf(
+		"package '%s' has more parts than provided directory path '%s'",
+		[concat(".", _pkg_path_values), concat("/", _file_path_values)],
+	)
+}
+
+# METADATA
+# description: emit notice when single file is provided, but with no severity
+notices contains _notice(message, "none") if {
+	count(_file_path_values) == 0
+
+	message := "provided file has no directory components in its path... try linting a directory"
+}
+
+report contains violation if {
+	# get the last n components from file path, where n == count(_pkg_path_values)
+	file_path_length_matched := array.slice(
+		_file_path_values,
+		count(_file_path_values) - count(_pkg_path_values),
+		count(_file_path_values),
+	)
+
+	file_path_length_matched != _pkg_path_values
+
+	not _known_file_path_matches(file_path_length_matched, _pkg_path_values)
+
+	violation := result.fail(rego.metadata.chain(), result.location(input["package"].path))
+}
+
+_pkg_path := [p.value |
+	some i, p in input["package"].path
+	i > 0
+]
+
+_pkg_path_values := without_test_suffix if {
+	cfg := config.for_rule("idiomatic", "directory-package-mismatch")
+
+	cfg["exclude-test-suffix"]
+
+	without_test_suffix := array.concat(
+		array.slice(_pkg_path, 0, count(_pkg_path) - 1),
+		[trim_suffix(regal.last(_pkg_path), "_test")],
+	)
+}
+
+_file_path_values := array.slice(parts, 0, count(parts) - 1) if {
+	parts := split(input.regal.file.name, input.regal.environment.path_separator)
+}
+
+# when a directory path, like `bar/baz`, is shorter than the package
+# path, like `foo.bar.baz` this function returns true when the last
+# "known" paths match, i.e. in this case `bar/baz` and `bar.baz`
+_known_file_path_matches(file_path, pkg_path) if {
+	diff := count(pkg_path) - count(file_path)
+
+	diff > 0
+	array.slice(pkg_path, diff, count(pkg_path)) == file_path
+}
+
+_notice(message, severity) := {
+	"category": "idiomatic",
+	"description": message,
+	"level": "notice",
+	"title": "directory-package-mismatch",
+	"severity": severity,
+}

bundle/regal/rules/idiomatic/directory-package-mismatch/directory_package_mismatch_test.rego

@@ -0,0 +1,80 @@
+package regal.rules.idiomatic["directory-package-mismatch_test"]
+
+import rego.v1
+
+import data.regal.config
+
+import data.regal.rules.idiomatic["directory-package-mismatch"] as rule
+
+test_success_directory_structure_package_path_match if {
+	module := regal.parse_module("foo/bar/baz/p.rego", "package foo.bar.baz")
+	r := rule.report with input as module with config.for_rule as default_config
+
+	r == set()
+}
+
+test_fail_directory_structure_package_path_mismatch if {
+	module := regal.parse_module("foo/bar/baz/p.rego", "package foo.baz.bar")
+	r := rule.report with input as module with config.for_rule as default_config
+
+	r == with_location({"col": 9, "file": "foo/bar/baz/p.rego", "row": 1, "text": "package foo.baz.bar"})
+}
+
+test_success_directory_structure_package_path_match_longer_directory_path if {
+	module := regal.parse_module("system/directories/foo/bar/baz/p.rego", "package foo.bar.baz")
+	r := rule.report with input as module with config.for_rule as default_config
+
+	r == set()
+}
+
+# note that this is not considered a violation but a _notice_ of severity warning
+# see corresponding test below
+test_success_directory_structure_package_path_match_shorter_directory_path if {
+	module := regal.parse_module("bar/baz/p.rego", "package foo.bar.baz")
+	r := rule.report with input as module with config.for_rule as default_config
+
+	r == set()
+}
+
+test_notice_severity_warning_when_directory_path_shorter_than_package_path if {
+	module := regal.parse_module("bar/baz/p.rego", "package foo.bar.baz")
+	r := rule.notices with input as module with config.for_rule as default_config
+
+	r == {{
+		"category": "idiomatic",
+		"description": "package 'foo.bar.baz' has more parts than provided directory path 'bar/baz'",
+		"level": "notice",
+		"severity": "warning",
+		"title": "directory-package-mismatch",
+	}}
+}
+
+test_notice_severity_none_when_no_path_likely_single_file_provided if {
+	module := regal.parse_module("p.rego", "package p")
+	r := rule.notices with input as module with config.for_rule as default_config
+
+	r == {{
+		"category": "idiomatic",
+		"description": "provided file has no directory components in its path... try linting a directory",
+		"level": "notice",
+		"severity": "none",
+		"title": "directory-package-mismatch",
+	}}
+}
+
+with_location(location) := {{
+	"category": "idiomatic",
+	"description": "Directory structure should mirror package",
+	"level": "error",
+	"location": location,
+	"related_resources": [{
+		"description": "documentation",
+		"ref": config.docs.resolve_url("$baseUrl/$category/directory-package-mismatch", "idiomatic"),
+	}],
+	"title": "directory-package-mismatch",
+}}
+
+default_config := {
+	"level": "error",
+	"exclude-test-suffix": true,
+}

docs/rules/idiomatic/directory-package-mismatch.md

@@ -0,0 +1,129 @@
+# directory-package-mismatch
+
+**Summary**: Directory structure should mirror package
+
+**Category**: Idiomatic
+
+**Automatically fixable**: Yes
+
+## Rationale
+
+Quickly finding the package you're looking for in a policy repository is made much easier when package paths are
+mirrored in the directory structure of your project. Meaning that if the name of your package is
+`permissions.users.claims`, it should reside in a file under the `permissions/users/claims` directory. Note however
+that any number of files can contribute to the same package! The `permissions/users/claims` directory may thus
+contain several policy files that all declare `package permissions.users.claims`.
+
+### Example
+
+An example of directory structure for a project following this convention might look like this:
+
+```shell
+# Directory structure                   # Package path
+.
+├── README.md
+└── bundle
+    └── authorization
+        ├── main.rego                   # authorization
+        └── rbac
+            ├── data.json               # authorization.rbac
+            ├── roles
+            │   └── roles.rego          # authorization.rbac.roles
+            │   └── roles_test.rego     # authorization.rbac.roles_test
+            └── users
+                ├── customers.rego      # authorization.rbac.users
+                ├── customers_test.rego # authorization.rbac.users_test
+                ├── internal.rego       # authorization.rbac.users
+                └── internal_test.rego  # authorization.rbac.users_test
+```
+
+### Tests
+
+Astute observers may notice that the test files in the example above are placed in the same directory as the
+policies they test. This may seem to contradict the
+[test-outside-test-package](https://docs.styra.com/regal/rules/testing/test-outside-test-package) rule, which
+says that any test package should have a `_test` suffix in its package path. However, putting tests next to
+the file they target arguably makes it _easier_ to find, and is a common practice in the OPA community. This
+rule therefore by default ignores the `_test` suffix when determining whether the package path matches the
+directory structure.
+
+This behavior can be changed by setting the `exclude-test-suffix` configuration option to `false`, in which
+case package paths with a `_test` suffix also will be required to reside in a directory with a `_test` suffix.
+Notably, this directory structure is used (and enforced) by [Styra DAS](https://docs.styra.com/das) and projects
+that integrate with it.
+
+Setting the `exclude-test-suffix` option to `false` means the example from above would now look like this:
+
+```shell
+# Directory structure                   # Package path
+.
+├── README.md
+└── bundle
+    └── authorization
+        ├── main.rego                   # authorization
+        └── rbac
+            ├── data.json               # authorization.rbac
+            ├── roles
+            │   └── roles.rego          # authorization.rbac.roles
+            ├── roles_test
+            │   └── roles_test.rego     # authorization.rbac.roles_test
+            ├── users
+            │   ├── customers.rego      # authorization.rbac.users
+            │   └── internal.rego       # authorization.rbac.users
+            └── users_test
+                ├── customers_test.rego # authorization.rbac.users_test
+                └── internal_test.rego  # authorization.rbac.users_test
+```
+
+Whichever way you choose is up to you. Consistency is key!
+
+### Bundles
+
+While directory structure doesn't matter to OPA when parsing _policies_, directories parsed as
+[bundles](https://www.openpolicyagent.org/docs/latest/management-bundles/) will read _data_ (`data.json` or
+`data.yaml`) files and insert the data in the `data` document tree based on the directory structure relative
+to the bundle root. Having policies structured in the same manner provides a uniform experience, and makes it
+easier to understand where both policies and data come from.
+
+### Editor Support
+
+Editors integrating Regal's [language server](https://docs.styra.com/regal/language-server) will automatically display
+suggestions for idiomatic package paths based on the directory structure in which a policy resides. The image below
+demonstrates a new policy being created inside an `authorization/rbac/roles` directory, and the editor
+([via Regal](https://docs.styra.com/regal/language-server#code-completions)) suggesting the package path
+ `authorization.rbac.roles`.
+
+<img
+  src={require('../../assets/rules/pkg_name_completion.png').default}
+  alt="Package path auto-completion in VS Code"/>
+
+## Configuration Options
+
+This linter rule provides the following configuration options:
+
+```yaml
+rules:
+  idiomatic:
+    directory-package-mismatch:
+      # one of "error", "warning", "ignore"
+      level: error
+      # exclude _test suffixes from package paths before comparing
+      # them to directory structure paths. when set to false, a
+      # package like authz.policy_test would need to be placed in
+      # an authz/policy_test directory, and if set to true (default)
+      # would be expected to be in authz/policy
+      exclude-test-suffix: true
+```
+
+## Related Resources
+
+- Rego Style Guide: [Package name should match file location](https://docs.styra.com/opa/rego-style-guide#package-name-should-match-file-location)
+- Regal Docs: [test-outside-test-package](https://docs.styra.com/regal/rules/testing/test-outside-test-package)
+- OPA Docs: [Bundles](https://www.openpolicyagent.org/docs/latest/management-bundles/)
+- Styra Docs: [Styra DAS](https://docs.styra.com/das)
+
+## Community
+
+If you think you've found a problem with this rule or its documentation, would like to suggest improvements, new rules,
+or just talk about Regal in general, please join us in the `#regal` channel in the Styra Community
+[Slack](https://communityinviter.com/apps/styracommunity/signup)!