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.

Sign up for GitHub

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

Jump to bottom

OPA build/fmt panics on second line comment when using | #5798

Closed
Djoust opened this issue Mar 30, 2023 · 0 comments
Closed

OPA build/fmt panics on second line comment when using | #5798

Djoust opened this issue Mar 30, 2023 · 0 comments
Labels
bug

Comments

@Djoust
Copy link

Djoust commented Mar 30, 2023

Short description

OPA build/fmt panics on second line comment when using |

Expected behavior

This should work correctly, when runnig opa build or opa fmt:

package test

rule01 = fail
{
    fail = {                # this
        x |                 # panics
            set[x]; f(x)
    }
}

PS: Error undefined function f is expected when compiled and run. Code is an example for the bug in fmt

Actual Behavior

opa fmt returns a panic:

opa fmt test.rego 
panic: overwriting non-nil beforeEnd

goroutine 1 [running]:
github.com/open-policy-agent/opa/format.(*writer).beforeLineEnd(...)
        /Users/runner/work/opa/opa/format/format.go:1294
github.com/open-policy-agent/opa/format.(*writer).insertComments(0xc00019b6e0, {0xc000198530, 0x1, 0x13?}, 0xc00049aa40)
        /Users/runner/work/opa/opa/format/format.go:497 +0x2e5
github.com/open-policy-agent/opa/format.(*writer).writeTermParens(0xc00019b6e0, 0x70?, 0xc00013ea20, {0xc000198530?, 0x101778680?, 0x10001ab01?})
        /Users/runner/work/opa/opa/format/format.go:680 +0x57
github.com/open-policy-agent/opa/format.(*writer).writeTerm(...)
        /Users/runner/work/opa/opa/format/format.go:676
github.com/open-policy-agent/opa/format.(*writer).writeComprehension(0xc00019b6e0, 0x6d?, 0xf4?, 0xc00013ea20, {0xc000704d10, 0x2, 0x2}, 0xc00049ad40, {0xc000198530, 0x1, ...})
        /Users/runner/work/opa/opa/format/format.go:888 +0xab
github.com/open-policy-agent/opa/format.(*writer).writeSetComprehension(0xc00019b6e0, 0xc00027ef20, 0xc00027ef20?, {0xc000198530, 0x1, 0x1})
        /Users/runner/work/opa/opa/format/format.go:864 +0x10c
github.com/open-policy-agent/opa/format.(*writer).writeTermParens(0xc00019b6e0, 0x98?, 0xc00013e8d0, {0xc000198528?, 0xc0003bf248?, 0x101c44740?})
        /Users/runner/work/opa/opa/format/format.go:699 +0x265
github.com/open-policy-agent/opa/format.(*writer).writeTerm(...)
        /Users/runner/work/opa/opa/format/format.go:676
github.com/open-policy-agent/opa/format.(*writer).writeFunctionCall(0xc00019b6e0, 0xc000198518?, {0xc000198520, 0x1, 0x1})
        /Users/runner/work/opa/opa/format/format.go:640 +0x3c5
github.com/open-policy-agent/opa/format.(*writer).writeExpr(0xc00019b6e0, 0xc0001bc5a0, {0xc000198518?, 0x102628360?, 0xc00049a900?})
        /Users/runner/work/opa/opa/format/format.go:541 +0x13d
github.com/open-policy-agent/opa/format.(*writer).writeBody(0xc000704ed0?, {0xc000198330, 0x1, 0xc00019b6e0?}, {0xc000704ee0?, 0x100700000?, 0xc000198508?})
        /Users/runner/work/opa/opa/format/format.go:519 +0x86
github.com/open-policy-agent/opa/format.(*writer).writeRule(0xc00019b6e0, 0xc0001bc550, 0x2?, {0x0?, 0x0?, 0x0?}, {0xc000704ed0, 0x2, 0x2})
        /Users/runner/work/opa/opa/format/format.go:358 +0x43f
github.com/open-policy-agent/opa/format.(*writer).writeRules(0x0?, {0xc000198500, 0x1, 0x102628360?}, {0xf0?, 0x82?, 0x19?}, {0xc000704ec0, 0x2, 0x2})
        /Users/runner/work/opa/opa/format/format.go:305 +0x9f
github.com/open-policy-agent/opa/format.(*writer).writeModule(0xc00019b6e0, 0xc00005ae00?, {0xa0?, 0xeb?, 0x13?})
        /Users/runner/work/opa/opa/format/format.go:272 +0x465
github.com/open-policy-agent/opa/format.AstWithOpts({0x101778740?, 0xc00005ad80?}, {0x0?})
        /Users/runner/work/opa/opa/format/format.go:152 +0x5e6
github.com/open-policy-agent/opa/format.Ast(...)
        /Users/runner/work/opa/opa/format/format.go:61
github.com/open-policy-agent/opa/format.Source({0x7ff7bfeff83a, 0x9}, {0xc0001b2000?, 0x0?, 0x0?})
        /Users/runner/work/opa/opa/format/format.go:40 +0xa5
github.com/open-policy-agent/opa/cmd.formatFile(0x102661ee0, {0x101c4ab20, 0xc000198008}, {0x7ff7bfeff83a, 0x9}, {0x101c56ad0, 0xc00072f450}, {0x0?, 0x0?})
        /Users/runner/work/opa/opa/cmd/fmt.go:111 +0x28d
github.com/open-policy-agent/opa/cmd.opaFmt.func1({0x7ff7bfeff83a?, 0xc0003bfc20?}, {0x101c56ad0?, 0xc00072f450?}, {0x0?, 0x0?})
        /Users/runner/work/opa/opa/cmd/fmt.go:76 +0x5f
path/filepath.walk({0x7ff7bfeff83a, 0x9}, {0x101c56ad0, 0xc00072f450}, 0x101afe028)
        /Users/runner/hostedtoolcache/go/1.20.2/x64/src/path/filepath/path.go:480 +0x116
path/filepath.Walk({0x7ff7bfeff83a, 0x9}, 0x101afe028)
        /Users/runner/hostedtoolcache/go/1.20.2/x64/src/path/filepath/path.go:571 +0x6c
github.com/open-policy-agent/opa/cmd.opaFmt({0xc000704a60?, 0x1, 0x0?})
        /Users/runner/work/opa/opa/cmd/fmt.go:75 +0xfa
github.com/open-policy-agent/opa/cmd.glob..func1(0x10260c440?, {0xc000704a60?, 0x1?, 0x1?})
        /Users/runner/work/opa/opa/cmd/fmt.go:53 +0x27
github.com/spf13/cobra.(*Command).execute(0x10260c440, {0xc000704a20, 0x1, 0x1})
        /Users/runner/work/opa/opa/vendor/github.com/spf13/cobra/command.go:920 +0x847
github.com/spf13/cobra.(*Command).ExecuteC(0x10260c160)
        /Users/runner/work/opa/opa/vendor/github.com/spf13/cobra/command.go:1044 +0x3bd
github.com/spf13/cobra.(*Command).Execute(0xc0000061a0?)
        /Users/runner/work/opa/opa/vendor/github.com/spf13/cobra/command.go:968 +0x19
main.main()
        /Users/runner/work/opa/opa/main.go:14 +0x25

Steps To Reproduce

Save the above code to test.rego

run:
opa fmt test.rego

Additional context

OPA Version: 0.50.0

Related issue:
#3836
@Djoust Djoust added the bug label Mar 30, 2023
rumenvasilev added a commit to rumenvasilev/opa that referenced this issue Mar 30, 2023
@rumenvasilev
add issue open-policy-agent#5798 to test cases
355a15d
Trolloldem added a commit to Trolloldem/opa that referenced this issue May 30, 2023
@Trolloldem
fmt:fix fmt panic in comprehension with comments
a165b4b 
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 open-policy-agent#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 open-policy-agent#3864.

Fixes: open-policy-agent#5798

Signed-off-by: Gianluca Oldani <oldanigianluca@gmail.com>
@Trolloldem Trolloldem mentioned this issue May 30, 2023
Merged
ashutosh-narkar pushed a commit to ashutosh-narkar/opa that referenced this issue Jun 5, 2023
@Trolloldem @ashutosh-narkar
fmt:fix fmt panic in comprehension with comments
88099dd 
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 open-policy-agent#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 open-policy-agent#3864.

Fixes: open-policy-agent#5798

Signed-off-by: Gianluca Oldani <oldanigianluca@gmail.com>
(cherry picked from commit 917dfc4)
ashutosh-narkar pushed a commit that referenced this issue Jun 6, 2023
@Trolloldem @ashutosh-narkar
fmt:fix fmt panic in comprehension with comments
59e1ac0 
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>
(cherry picked from commit 917dfc4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
Open Policy Agent
Status: Done
Milestone
No milestone
Development

No branches or pull requests
1 participant
@Djoust