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.

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

The sequential example in the docs doesn't work #156

Closed
jonathan-s opened this issue Apr 4, 2024 · 6 comments
Closed

The sequential example in the docs doesn't work #156

jonathan-s opened this issue Apr 4, 2024 · 6 comments

Comments

@jonathan-s
Copy link

jonathan-s commented Apr 4, 2024

When you try the following pattern described in the docs for sequential it doesn't work in the studio.

You also get the following warning.

Log Warning: sequential matches at the top of the file. If a pattern matched outside of a 
sequential, but no longer matches, it is likely because naked patterns are 
automatically wrapped with `contains bubble <pattern>`

This warning is somewhat unclear, as it doesn't really tell the user how they should proceed to make things work the way they expect it to.

@ilevyor
Copy link
Contributor

ilevyor commented Apr 4, 2024

good catch in the docs.

The error message is trying to explain that most patterns are auto wrapped

https://docs.grit.io/language/bubble#pattern-auto-wrap

sequential is not auto wrapped because it only matches on files, so the pattern inside of a sequential needs to match a top level file. in this case the easy fix is to insert a contains in front of each step like so:

sequential {    
    contains `console.log($message)` => `console.warn($message)`,
    contains `console.warn($message)` => `console.info($message)`
}

@jonathan-s
Copy link
Author

Are there other strategies besides using contains?

@morgante morgante closed this as completed Apr 4, 2024
@morgante
Copy link
Contributor

morgante commented Apr 4, 2024

Are there other strategies besides using contains?

The core thing is that each step is matching the file as a whole. In most cases, you want to look for some code inside the file so contains is the best option. Why do you want an alternative?

@jonathan-s
Copy link
Author

jonathan-s commented Apr 10, 2024

@morgante This may be another bug, or some misunderstanding from my side.

The following grit code.

sequential {
        bubble file($body) where $body <: contains `def $name($args, $...): $funcbody` where {
        $args <: `self` => .
    }
}

Seems to replace the first self in a function.

While the following grit code

`def $name($args, $...): $funcbody` where {
    $args <: `self` => .
}

replaces all the self in the python code of the file. So clearly just using the construct with bubble and contains are not equivalent.

Python code to use.

class AlertTest(BaseTest):
    @vcr.use_cassette
    def test_rule(self):
        resp = self.get("/url/alert/rule")
        self.assertEqual(resp.status_code, 200)

    @vcr.use_cassette
    def test_alert_search(self):
        resp = self.post(
            "/url/alert/search",
            data={
                "alert_rule": "oeuoeu",
                "use_case": "usecase",
                "panels": "count,rules,details",
            },
        )

        self.assertEqual(resp.status_code, 200)

@morgante
Copy link
Contributor

The bubble needs to go around the scope you actually want to isolate, like this:

sequential {
    file($body) where $body <: contains bubble `def $name($args, $...): $funcbody` where {
        $args <: `self` => .
    }
}

@jonathan-s
Copy link
Author

jonathan-s commented Apr 16, 2024

@morgante Thank you.
Worth noting that the example in documentation here > https://docs.grit.io/language/patterns#sequential

contains

sequential {
    bubble file($body) where $body <: contains `console.log($message)` => `console.warn($message)`,
    bubble file($body) where $body <: contains `console.warn($message)` => `console.info($message)`
}

In other words it is using bubble the very first.

Also I'm noticing that or does not seem to work together with bubble. Is there a way to make that work?

Ie the following code does not work as expected finding zero matches. Though each sequence in separation does work as expected.

sequential {
    file($body) where $body <: contains bubble `class $name($parent): $funcbody` => $funcbody,
    file($body) where $body <: contains bubble or {
        `self.assertEqual($first, $second)` => `assert $first == $second`,
        `self.assertEqual($first, $second, $third)` => `assert $first == $second`
    },
}

Some feedback regarding sequential. I'm finding the use of sequential pretty fiddly and it's hard to know exactly what's going on and why it's not working.

The way I conceptualize sequential is a loop, so that each statement is applied on the file in question. Right now if you have a series of statements and one of the statements doesn't match, nothing at all is changed on the file. That makes it difficult to debug, but also it may be desirable to apply the series of transformations and if one doesn't match it would still apply the other transformations.

I would rather get a warning message saying that the following statement was had no match in xxx file, and thus no transformation happened there.

Also it would be nice if no extra syntax was needed to stay consistent with single statement transformations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants