The parser was accumulating comments in a parser-global slice. If the
parser backtracked, the comments were not thrown away. Because most
successful parses seem to backtrack, this meant that most ASTs contained
duplicated comments.
This change simply modifies the parser to accumulate the comments in a
set that just gets sorted at the end.
Fixes#426
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
Fixes#1129 for the most part.
What still is wrongly put, but also harder to fix, is the case where
there's tabs INSIDE the line:
p = TAB true TAB { TAB as }
or a space before the tab, like
SPACE TAB p = true { as }
will still have a misaligned "^", as in that case, not all tabs are
fully expanded.
This merely trims leading tabs, and fixes the error maker location if
there's no other tabs used in the line.
However, that should fit common usage: Leading tabs is what `opa fmt`
proposes; and I'm doubtful of too many uses of tabs in other places
in Rego code.
Signed-off-by: Stephan Renatus <srenatus@chef.io>
The parser was panic-ing when constructing an error detail because it
could index into the line with a negative offset. These changes improve
the improve the checks against the position input.
Fixes#948
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
PreviousLy if a match failure occurred, the error would be returned to
the caller as is. Because of how the grammar is specified this often
resulted in noisy error messages that list all of the low level symbols
that the parser _tried_ to match (e.g., #, %, /, >, etc.) The expected
values are basically useless for users.
With these changes the expected values are removed from the parser
errors and the parser errors now include the text from the line in
question and a carrot identifying where the failure occurred.
These changes also improve the error hanlding for non-terminated strings
and other well-known classes of parser errors. In the future we can
improve the parser to return more specific errors that are more useful
than the "no match found" default.
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
Previously we had constants defined for AST type names. These were used
in error messages in various places. The original goal was to make error
messages consistent, however, this approach made it difficult to locate
the source of the error in code.
These changes allow calls to be nested inside terms (e.g., f(x) !=
g(x)). As part of these changes a few things have been refactored:
1) Grammar has been restructured so that construction code is pulled out
into a separate file. Hopefully this makes the grammar more readable.
2) String() implementation on Expr has been simplified to use prefix
notation for calls (except equality) as this avoids the challenge of
worrying about roundtripping policy strings (which is done frequently
inside test cases.) E.g., plus(x,1,y) converted to infix x + 1 = y would
parse to eq(x + 1, y).
Now that the AST walker can visit terms (as well as values) it's better
to just walk all terms in the statement and mangle them (rather than
relying on speical handling for collections of terms, which is bound to
break over time.)
Fixes#480
Previously, functions were implemented with a separate set of types that
had their own code paths in the compiler, eval, etc. These changes
refactor the function implementation so that functions are implemented
as rules with one or more arguments.
By representing functions as rules, we can avoid special casing required
to support functions, e.g., during parse and compile there are a number
of steps that required special casing for functions:
- Parser needed separate grammar definitions for functions (which
prevented them from being chained or using else)
- Compiler needed separate resolver and type checker implementations
which was a source of bugs.
In some cases, special casing is unavoidable for now (e.g., during eval)
however this could be improved in the future.
Fixes#471Fixes#467Fixes#463
These changes refactor the parser extensions that convert bodies into
rules if they can interpreted as such. The cases that can be converted
are clearer now and the test coverage is improved.
Fixes#433
These changes allow partial docs to be defined without a body in Rego
source files. Before, the rules would have to include a `{true}` body
for the parser to allow them. Now, the body can be omitted.
Rules defined this way (inside modules) cannot be copy/pasted as-is into
the REPL. This could be addressed by creating a "paste mode" in the REPL
similar to ipython and other interactive shells.
These changes build on https://github.com/open-policy-agent/opa/pull/412
with a few differences:
- Dynamic values are allowed in the head.
- Partial sets are allowed.
Both of these changes are based on personal experience writing policy.
Dynamic values are fine to allow as the compiler will catch unsafe vars
and rewrite the head to handle refs and comprehensions.
When the parser is post-processing parsed Rego, it takes steps to
transform variable references to the `data` and `input` root documents
into Refs with the variables as the head. It does this using the
Transform function of the ast package, which will proceed to transform
the new Ref as well. While it would always replace the ref head with
itself, the Go race detector correctly saw concurrent reads and writes
to the Ref as a data race. The parser extensions have been corrected to
replace variable instances of the root documents with separate copies of
their Ref counterparts, instead of using a single global instance of
each across calls.
When parsing, OPA transforms constants into rules with only the
expression `true` in the body. This is not desirable behavior, as
it's much cleaner for them to be expressed in the shorthand.
This patch also adds a test for parsing invalid rego.
OPA converts lone bodies to rules containing a single expression
in their bodies (true). However, the parser extension that did this
neglected to set the location of the new rule and the new expression.
There were a handful of places in the parser where the location was not
being set on terms. All parsed terms should have a location set (for
debug/error purposes.) These changes correct the missing cases in the
parser. Also, now that the visitor can process terms, we can add back
the filename setter on terms.
Before, the walker would skip the Term and move straight to the Term
value. In some cases, it's useful to be able to process Terms
generically.
Note, this required removing the filename setter as some terms do not
have a location set. This is a separate issue that will be fixed in the
next commit.
These changes update the parser, compiler, and related helpers to
support the else keyword.
These changes do not include the updates required for rule indexing.
ParseTerm should have been checking if body was exactly size 1. This was
allowing the form in index.html to trigger a panic if the input field
was empty.
Given a parsed rule, it should be easy to determine the path of the
document produced by that rule without having access to the entire that
the rule is contained in.
- Refactor error codes to use strings instead of ints.
- Simplify error messages throughout.
- Ensure location set on all expressions. There were a couple locations
in the parser/compiler where locations were not being set.
- Fallback to rule location in topdown in case location not set. This
ensures that users get useful locations for API requests with paths
that refer to virtual docs exactly.
Also add Find function to ast.Value. Useful for extracting values
dynamically. Eventually can support JSON pointers.
Fixes#237
This commit contains a few syntax/parsing changes:
1) Old head :- body rule syntax is no longer supported. All rules must
be declared as: head { body } now.
2) Semicolon has replaced comma as the conjunction operator. This avoids
the ambiguity between set literals and curly-brace enclosed bodies.
3) ast.ParseBody will concatenate bodies that it receives from the
parser. This way, callers can invoke ast.ParseBody without enclosing
multiple expressions in braces.
4) Trailing commas are allowed in sets, arrays, and objects. To declare
a set of size one a trailing comma must be used, e.g., {foo,} =>
set(foo) where as {foo} => body(expr(foo)).
The following commit will update all of the existing test cases to bring
them into compliance with the new syntax.
Fixes#253
As part of this change, comments have been added into the AST however they are
not stored on other AST nodes or the module for now (so this should not affect
any part of compilation or evaluation).
With the request document changes, the "request" var in "request = <term>"
is transformed into a ref. As a result, the body to rule conversion was not
working (and returning an error).
Fixes#202
The grammar was incorrectly attempting to match keywords to prevent them from
being used as variable names. Now, the parser includes a predicate function to
check that keywords are not used in place of variable names.
Fixes#178
Previously, the token "data" would be parsed into Var("data"). Because "data"
is a reserved word, it would pass the safety checks and then fail to evaluate.
With this change, Var("data") is transformed into Ref("data"). This means the
safety checks will catch the recursion (instead of failing during evaluation).
Previously, errors returned by the parser and compiler would encode the
location information in the error string. Now, the location information is
structured and can be readily used by consumers.
As mentioned in the comment, this is a workaround for an issue in the parser.
If the parser does not find a match for any of the rules, it returns a bogus
error location.
This will allow callers to unambiguously compare expressions contained in an
ast.Body. E.g., the query "p[x], p[x]" contains two separate but syntactically
equal expressions. The new field will allow callers to distinguish between
them.
Also, refactor code that manually constructs ast.Body. Instead, call NewBody
which will handle setting the new field correctly.
These changes add some helpers to obtain contextual information for error
mesasages. Specifically, the Location values on all AST nodes can be used to
format error messages that include line, column, and filename.
Also, add compiler step to check that built-ins are provided with the correct
number of arguments. The built-in implementations in topdown assume this is
the case and will index into the term slice without checking.
Torin Sandall