All packages, except for `cmd` and `internal`, have been moved into a new `v1` root package.
Old packages are kept for backwards-compatibility reasons. All contained code is replaced with simple type aliases and proxy functions to `v1` implementations.
Old packages default to the Rego v0 syntax, new `v1` packages default to the Rego v1 syntax.
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
**main**
```
BenchmarkLintAllEnabled-10 1 2640715625 ns/op 6385110200 B/op 116296633 allocs/op
```
**pr**
```
BenchmarkLintAllEnabled-10 1 2597179708 ns/op 6183614112 B/op 108421141 allocs/op
```
(I renamed the benchmark, but this is the same as "regal linting itself"
used in the past)
Another 8 million allocations cut off from `regal lint bundle`,
and a whopping 10% improvements to wall clock time!
The most significant improvement is the Equal implementation for
refs, since that is called all over the place. But there are many
other fixes here, and they all contribute something substantial
(and fixes that only have had marginal impact have been left out).
Signed-off-by: Anders Eknert <anders@styra.com>
The previous version has been failing without any good reason for me,
so let's try this.
About the version pick: It's not the latest version (v1.62.0 at the
moment), because that would introduce a new revive rule,
redeclares-builtin-id, and that flags every variable called `min` or
`max` in the code base. I had started addressing these, but they were
just too many.
The new issues related to this version are mostly that it complains
whenever it finds a non-static string that makes its way into a printf-
like function. However, that's a common pattern in some place here, so
I've sprinkled some nolint:govet on it.
Signed-off-by: Stephan Renatus <stephan@styra.com>
* Updating `compile` package tests
* Respect optimizer rego-version for optimized support modules
* Setting parser options to use module rego-version when `bundle.FormatModulesForRegoVersion()` should preserve parsed module rego-version
* Enforcing requested rego-version for partial-eval support modules
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
This fix preserves the brackets around set union operations
when the inflix operator (`|`) is used. Previously the formatter
would remove the brackets around the union operation, interpreting
the expression as a comprehesion which would yeild unexpected results.
Fixes: #6588
Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>
Also deprecating `ParserOptions.EffectiveRegoVersion()`, which will be removed in a future release.
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
* ast+cmd+rego: Adding `--rego-v1` flag to `opa eval`
Fixes: #6463
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
* Adding `--rego-v1` flag to `opa build`
Fixes: #6463
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
* Formatting PE support modules to comply with rego-v1 when required
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
* Removing rego.v1 import when formatting for rego-v1 (not rego-v0-compat-v1)
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
* touch up
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
* Fixing linting issues
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
* Consolidating `Bundle.FormatModules()` and `Bundle.FormatModulesForRegoVersion()`
Suggested by @ashutosh-narkar
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
* Adding descriptions to `RegoVersion`
Requested by @ashutosh-narkar
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
* Using `--v1-compatible` flag instead of `--rego-v1`
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
* Updating docs
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
* Reintroducing `ParserOptions.RegoV1Compatible` to avoid breaking change
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
* cmd & tester
Adding `--v1-compatible` flag to `opa test`
Fixes: #6463
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
* Adding `--v1-compatible` flag to `opa fmt`
Fixes: #6463
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
* Adding `--v1-compatible` flag to `opa check`
Fixes: #6463
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
* Making linter happy
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
* Review modifications suggested by @ashutosh-narkar
* Changing `ParserOptions.RegoV1Compatible` take precedence over `ParserOptions.RegoVersion`
* Fixing comment in test
* Updating `fmt --rego-v1` CLI description
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
* Review modifications suggested by @ashutosh-narkar
* Changing `ParserOptions.RegoV1Compatible` take precedence over `ParserOptions.RegoVersion`
* Fixing comment in test
* Updating `fmt --rego-v1` CLI description
* Adding back `Opts.RegoV1` and deprecating.
* Making `Opts.RegoV1` take precedence over `Opts.RegoVersion`
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
* Review modifications suggested by @ashutosh-narkar
* Changing `ParserOptions.RegoV1Compatible` take precedence over `ParserOptions.RegoVersion`
* Fixing comment in test
* Updating `fmt --rego-v1` CLI description
* Adding back `Opts.RegoV1` and deprecating.
* Making `Opts.RegoV1` take precedence over `Opts.RegoVersion`
* `TestPartialWitRegoV1` -> `TestPartialWithRegoV1`
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
* Review modifications suggested by @ashutosh-narkar
* Changing `ParserOptions.RegoV1Compatible` take precedence over `ParserOptions.RegoVersion`
* Fixing comment in test
* Updating `fmt --rego-v1` CLI description
* Adding back `Opts.RegoV1` and deprecating.
* Making `Opts.RegoV1` take precedence over `Opts.RegoVersion`
* `TestPartialWitRegoV1` -> `TestPartialWithRegoV1`
* removing `Println` in test
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
* Review modifications suggested by @ashutosh-narkar
* Changing `ParserOptions.RegoV1Compatible` take precedence over `ParserOptions.RegoVersion`
* Fixing comment in test
* Updating `fmt --rego-v1` CLI description
* Adding back `Opts.RegoV1` and deprecating.
* Making `Opts.RegoV1` take precedence over `Opts.RegoVersion`
* `TestPartialWitRegoV1` -> `TestPartialWithRegoV1`
* removing `Println` in test
* Updating docs with per-command behavioural descriptions for `--v1-compatible`.
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
---------
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
And add test cases using chained bodies, as that would previously
get mistaken for implicit return without chaining.
Fixes#6467
Signed-off-by: Anders Eknert <anders@styra.com>
This change renames the `future.compat` import to `rego.v1`.
The latter is clear that it's a declaration that
a module is compatible with a v1 version of OPA.
Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>
Adding `future.compat` import for enforcing strict-mode checks and additional `1.0` behavior for the module.
Fixes: #6247
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
This is especially nice for functions doing pattern matching on
equality in its arguments.
Previously, we'd rewrite:
```rego
f(1)
```
into
```rego
f(1) = true
```
Now, we'll leave the shorter form alone, while still respecting
explicit assignment, using either `=` or `:=`.
Signed-off-by: Anders Eknert <anders@eknert.com>
* Adding support for multiple variables at arbitrary locations in rule refs
* Updating type-checker to handle general ref heads
Fixes: #5993Fixes: #5994
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
The comment said the parser duplicated comments, but that does no
longer seem to be the case. All tests pass without this and I was
not able to reproduce it any other way.
Signed-off-by: Anders Eknert <anders@styra.com>
Priot to this change, the `fmt` command panicked when
rego files processed contained a comprehension written
on multiple lines with comments in these lines. An
example of such comprehension is presented in #5798.
After this change, comprehensions on multiple lines are
not moved in a single line, in order to correcly handle
comments, similarly to what has been already done in #3864.
Fixes: #5798
Signed-off-by: Gianluca Oldani <oldanigianluca@gmail.com>
Prior to this change, the `fmt` command produced
invalid code when applied to the rego files
presented in #5537.
With these changes, the `fmt` command is now
able to handle the edge cases that produce
invalid code in output.
Fixes: #5537
Signed-off-by: Gianluca Oldani <oldanigianluca@gmail.com>
This commit fixes the index out of range error discussed in #5646 and adds error
handling to avoid that `fmt` panics when such errors are encountered by the formatter
The error handling procedure introduced is similar to the one used by the Scanner struct
responsible for the parsing of a `rego` file, since it uses a slice which is filled with
all the eventual errors that can be found during the format procedure
Fixes: #5646
Signed-off-by: Gianluca Oldani <oldanigianluca@gmail.com>
Before, we'd end up formatting
ps["foo"] = "bar" { true }
as
ps.foo = "bar" { true }
and older OPA version know how to parse the former, but not
the latter.
Fixes#5449.
Also includes:
* format: pass internal options via struct; because adding a third (in some cases
fifth) boolean argument just didn't seem right.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
This change allows rules to have string prefixes in their heads -- we've
come to call them "ref heads".
String prefixes means that where before, you had
package a.b.c
allow = true
you can now have
package a
b.c.allow = true
This allows for more concise policies, and different ways to structure
larger rule corpuses.
Backwards-compatibility:
- There are code paths that accept ast.Module structs that don't necessarily
come from the parser -- so we're backfilling the rule's Head.Reference
field from the Name when it's not present.
This is exposed through (Head).Ref() which always returns a Ref.
This also affects the `opa parse` "pretty" output:
With x.rego as
package x
import future.keywords
a.b.c.d if true
e[x] if true
we get
$ opa parse x rego
module
package
ref
data
"x"
import
ref
future
"keywords"
rule
head
ref
a
"b"
"c"
"d"
true
body
expr index=0
true
rule
head
ref
e
x
true
body
expr index=0
true
Note that
Name: e
Key: x
becomes
Reference: e[x]
in the output above (since that's how we're parsing it, back-compat edge cases aside)
- One special case for backcompat is `p[x] { ... }`:
rule | ref | key | value | name
------------------------+-------+-----+-------+-----
p[x] { ... } | p | x | nil | "p"
p contains x if { ... } | p | x | nil | "p"
p[x] if { ... } | p[x] | nil | true | ""
For interpreting a rule, we now have the following procedure:
1. if it has a Key, it's a multi-value rule; and its Ref defines the set:
Head{Key: x, Ref: p} ~> p is a set
^-- we'd get this from `p contains x if true`
or `p[x] { true }` (back compat)
2. if it has a Value, it's a single-value rule; its Ref may contain vars:
Head{Ref: p.q.r[s], Value: 12} ~> body determines s, `p.q.r.[s]` is 12
^-- we'd get this from `p.q.r[s] = 12 { s := "whatever" }`
Head{Key: x, Ref: p[x], Value: 3} ~> `p[x]` has value 3, `x` is determined
by the rule body
^-- we'd get this from `p[x] = 3 if x := 2`
or `p[x] = 3 { x := 2 }` (back compat)
Here, the Key isn't used, it's present for backwards compatibility: for ref-
less rule heads, `p[x] = 3` used to be a partial object: key x, value 3,
name "p"
- The destinction between complete rules and partial object rules disappears.
They're both single-value rules now.
- We're now outputting the refs of the rules completely in error messages, as
it's hard to make sense of "rule r" when there's rule r in package a.b.c and
rule b.c.r in package a.
Restrictions/next steps:
- Support for ref head rules in the REPL is pretty poor so far. Anything that
works does so rather accidentally. You should be able to work with policies
that contain ref heads, but you cannot interactively define them.
This is because before, we'd looked at REPL input like
p.foo.bar = true
and noticed that it cannot be a rule, so it's got to be a query. This is no
longer the case with ref heads.
- Currently vars in Refs are only allowed in the last position. This is expected
to change in the future.
- Also, for multi-value rules, we can not have a var at all -- so the following
isn't supported yet:
p.q.r[s] contains t if { ... }
-----
Most of the work happens when the RuleTree is derived from the ModuleTree -- in
the RuleTree, it doesn't matter if a rule was `p` in `package a.b.c` or `b.c.p`
in `package a`.
As such, the planner and wasm compiler hasn't seen that many adaptations:
- We're putting rules into the ruletree _including_ the var parts, so
p.q.a = 1
p.q.[x] = 2 { x := "b" }
end up in two different leaves:
p
`-> q
`-> a = 1
`-> [x] = 2`
- When planing a ref, we're checking if a rule tree node's children have
var keys, and plan "one level higher" accordingly:
Both sets of rules, p.q.a and p.q[x] will be planned into one function
(same as before); and accordingly return an object {"a": 1, "b": 2}
- When we don't have vars in the last ref part, we'll end up planning
the rules separately. This will have an effect on the IR.
p.q = 1
p.r = 2
Before, these would have been one function; now, it's two. As a result,
in Wasm, some "object insertion" conflicts can become "var assignment
conflicts", but that's in line with the now-new view of "multi-value"
and "single-value" rules, not partial {set/obj} vs complete.
* planner: only check ref.GroundPrefix() for optimizations
In a previous commit, we've only mapped
p.q.r[7]
as p.q.r; and as such, also need to lookup the ref
p.q.r[__local0__]
via p.q.r
(I think. Full disclosure: there might be edge cases here that are unaccounted
for, but right now, I'm aiming for making the existing tests green...)
New compiler stage:
In the compiler, we're having a new early rewriting step to ensure that the
RuleTree's keys are comparible. They're ast.Value, but some of them cause us
grief:
- ast.Object cannot be compared structurally; so
_, ok := map[ast.Value]bool{ast.NewObject([2]*ast.Term{ast.StringTerm("foo"), ast.StringTerm("bar")}): true}[ast.NewObject([2]*ast.Term{ast.StringTerm("foo"), ast.StringTerm("bar")})]
`ok` will never be true here.
- ast.Ref is a slice type, not hashable, so adding that to the RuleTree would
cause a runtime panic:
p[y.z] { y := input }
is now rewritten to
p[__local0__] { y := input; __local0__ := y.z }
This required moving the InitLocalVarGen stage up the chain, but as it's still
below ResolveRefs, we should be OK.
As a consequence, we've had to adapt `oracle` to cope with that rewriting:
1. The compiler rewrites rule head refs early because the rule tree expects
only simple vars, no refs, in rule head refs. So `p[x.y]` becomes
`p[local] { local = x.y }`
2. The oracle circles in on the node it's finding the definition for based
on source location, and the logic for doing that depends on unaltered
modules.
So here, (2.) is relaxed: the logic for building the lookup node stack can
now cope with generated statements that have been appended to the rule bodies.
There is a peculiarity about ref rules and extents:
See the added tests: having a ref rule implies that we get an empty object
in the full extent:
package p
foo.bar if false
makes the extent of data.p: {"foo": {}}
This is somewhat odd, but also follows from the behaviour we have right now
with empty modules:
package p.foo
bar if false
this also gives data.p the extent {"foo": {}}.
This could be worked around by recording, in the rule tree, when a node was
added because it's an intermediary with no values, but only children.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
This follows the same rules as 'if' used with ordinary rules:
1. if the future keyword is present, 'if' will be used in `opa fmt`'s output
2. shorthands are allowed:
p := true if 2>1
else := "blah" if 1 < 0
3. the formatter will only use the shorthand if the body was on one line with
the rest before:
else := 1 { whatever }
becomes
else := 1 if whatever
but
else := 1 {
whatever
}
becomes
else := 1 if {
whatever
}
Fixes#5002.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
This commit adds the `prealloc` linter to the list of linters for OPA, and fixes up the miscellaneous locations in the code that the linter found where we could easily preallocate slices.
Signed-off-by: Philip Conrad <philipaconrad@gmail.com>
`contains` provides an alternative way to declare partial sets:
p contains x {
x := { "foo": "bar"
}
which is the same as
p[x] {
x := { "foo": "bar"
}
The keyword is enabled by importing `future.keywords.contains`, and
when it _is enabled_, the format will be used for all partial sets in
that file for pretty-printing.
`if` is a new keyword allowing for more readable rule definitions:
The syntax is
NAME [if] { EXPR [EXPR...] }
and the is a shorthand allows dropping the braces around the expression
if there is only one:
NAME if EXPR
For example, this allows expressions like
allow if not deny
f(xs) if every x in xs { x != "foo" }
The one exception here are partial sets: they cannot use `if` UNLESS
they use `contains`:
p[x] { x := "foo" } # valid
p contains x { x := "bar" } # valid
p contains x if { x := "bar" } # valid
p[x] if { x := "foo" } # invalid
This is because we want to interpret that differently (as an object
rule defining `p.foo = true`) in the near future.
The formatter works in the same way: if `future.keywords.if` is imported, it
will be used where it can be used.
We don't want to be too eager when it comes to introducing syntactic sugar.
So this will be rewritten, because head and body expression are on the same
line:
p := 5 if { time.day_of_week() == "Monday" }
# => p := 5 if time.day_of_week() == "Monday"
but this won't:
p := 5 if {
time.day_of_week() == "Monday"
}
The rationale here is that if the policy author decided that they want this on
an extra line, we won't mess with it.
This also sidesteps the need to check if both the head and the single body
expression have a comment.
This change includes various docs updates. Notable exceptions are the GK docs,
since it will take a while for these keywords to be come available there; and
the frontpage: merging a PR would update the frontpage immediately, and we
don't want to show something there that isn't available in the latest release.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
Running the following through `opa fmt`:
package foo
bar {
# before
input.bar
# after
}
Causes the `after` comment to be moved outside of the rule:
package foo
bar {
# before
input.bar
}
# after
This was caused by `skipPast` in `closingLoc` being called even when there is no
`[key]` part in the rule head. Adding a third clause fixed this; it seems
like `closingLoc` is designed to take `0` in this case because of the
`skipOpen > 0`.
This did affect one other test case, where I had to add an extra newline
to separate the comment from the rule head. Without that, `insertComments`
(correctly, I guess) inserts:
} # some special case
Signed-off-by: Jasper Van der Jeugt <m@jaspervdj.be>
* format: allow ignoreing source locations
* cmd/eval: format disregarding source locations for partial result
Before, we'd see this output:
```
$ opa eval -p -fsource 'time.clock(input.x)==time.clock(input.y)'
# Query 1
time.clock(time.clock(input.x), input.y)
```
Now, we get the proper answer: `time.clock(input.y, time.clock(input.x))`.
Note that it's a _display_ issue; the JSON output of PE has not been affected.
Fixes#4609.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
Also ensure that added imports have a location set.
Previously, `opa fmt` on the added test file would have panicked
because the import hadn't had a location.
Fixes#4606.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
Do note though that this does not change how multiple
with statements are grouped. Although I agree with that,
it's IMHO a separate feature request, while the spacing
issue is a bug.
Signed-off-by: Anders Eknert <anders@eknert.com>
Importing `future.keywords.every` will ALSO import `future.keywords.in`,
since the latter is required for the former.
This includes the formatting of the expression itself, and adding
the "future.keywors.every" import if necessary:
This would happen when pretty-printing an AST that was parsed
with ast.ParserOptions enabling the required future keyword:
The import would not be present in the *ast.Module, but it would
be required to parse the pretty-printed result.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
As mentioned in the comment, empty file names happen when the format
package's Ast() function does a sweep of its input, and adds a
"default location" to everything that has a nil location.
During PE, when generated the pairs to save in saveUnify, we'll
return Var Terms without locations. Fixing that seemed like a bigger
hurdle, so I went this route.
The new check is such that if any term has the default file in
its location, such as would happen if we're formatting code that
was created programmatically (not parsed), we'll group the terms'
elements, but print them in one line.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
* ast: add 'every' future keyword, parser support, scaffolding
With this commit, "every x in xs { ... }" and "every k, v in { ... }"
will be parsed into a new struct, which is basically
Every {
Key, Value *Term
Domain *Term
Body Body
}
This includes the required to changes to visitors, comparisons, copy, ...
all the ceremony required to introduce a new keyword.
* format: format every statements
* ast: hide 'every' from capabilities for now
With this change,
- capabilities.json will NOT mention "every"
- "import future.keywords" will NOT get you "every"
- "import future.keywords.every" will complain about "every" being unknown
In tests, we're passing an unexported field in `ast.ParserOptions`,
which makes the parser treat "every" like it would eventually be
treated when unveiled.
The formatting tests are bluntly SKIPPED for now, to be re-enabled later.
---------------
NB: The work on "every" is ongoing.
Rewriting and evaluation follows. When it's documented, we'll unhide it.
---------------
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
When a module is formatted that has calls to `internal.member_2` or
`internal.member_3`, which get pretty-printed as infix `in` operator
calls, the formatter now ensures that the corresponding future keyword
import is present.
Fixes#4111.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
The current version of OPA shows unexpected behavior when formatting files that
contain operators in refs. This is commonly used to get e.g. the last item out
of an array:
$ cat test.rego
package test
foo = x {
arr = [1, 2, 3]
x = arr[count(arr) - 1]
}
$ opa fmt test.rego
package test
foo = x {
arr = [1, 2, 3]
x = arr[minus(count(arr), 1)]
}
This fixes that issue by using `writeTerm()` rather than `String()` for the ref.
Another approach could be to change this in `String()`; I wasn't sure which one
was better.
Signed-off-by: Jasper Van der Jeugt <jasper@fugue.co>
This snippet,
r = contains(
input.x,
"y",
)
would have been formatted as
r = contains(input.x, "y")
before. Now, any new lines added between function arguments will be kept, and
the snippet will not be reformatted.
As a consequence, comments on the separate arguments we OK:
r = contains(
input.x, # haystack
"y", # needle
)
and don't freak out the formatter.
Fixes#3836.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
* format: make groupIterable sort by row
Before, it depended on the elements being passed in ordered by their rows.
Before https://github.com/open-policy-agent/opa/pull/3823, the iteration
order was the same as the row order; but with sorting the keys slice (which
determines iteration order) on creation, that was changed.
Now, we'll sort the elements within `groupIterable`.
Fixes#3849.
Also includes:
* ast/term_bench_test: fix benchmark
Since map iteration is randomized in golang, this benchmark didn't
actually measure what was intended, but rather the presence or ab-
sence of duplicate keys.
Now, we'll create a set of keys to insert before, and either shuffle
it or use it in its increasing order: to call `(Object).Insert()` in
a loop.
* CHANGELOG/capabilities: update for v0.33.1 bugfix release
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
golint is deprecated. The author of the code no longer supports the
codebase. golangci-lint is faster than golint, and is in use by other
opa repositories (e.g. Gatekeeper).
This commit changes tools.go to reference golangci (so it ends up in
vendor) and modifies check-lint to use golangci instead.
Breaking API Changes:
- plugins/rest/rest.go: Fix typo "AllowInsureTLS" -> "AllowInsecureTLS"
- storage/errors.go: Removed unused IndexingNotSupportedErr
Signed-off-by: Will Beason <willbeason@google.com>
Anders Eknert