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

False positive on F1 rule with a derived table #32

Open
MartinGuindon opened this issue Jan 30, 2020 · 4 comments
Open

False positive on F1 rule with a derived table #32

MartinGuindon opened this issue Jan 30, 2020 · 4 comments

Comments

@MartinGuindon
Copy link

LAMS is reporting a false positive on the F1 rule for a derived table using parameters.

Detailed error: linkback references another view, if linkback_type, via {% if linkback_type._parameter_value == "Client" %}

The full dimension code is:

  dimension: linkback {
    sql: TRUE ;;
    html:
      {% if linkback_type._parameter_value == "Client" %}
      <p>Thank you for logging in!</p>
      <p><a href="https://<redacted>/client/{{ client_id._parameter_value | replace:'$','-' }}">Return to dashboard</a></p>
      {% endif %}
    ;;
  }
fabio-looker added a commit that referenced this issue Feb 5, 2020
@fabio-looker
Copy link
Contributor

FYI, I started working on this for about 30 minutes, but I'm overlooking something really obvious and am failing to correctly set up a failing test so far. Putting the link here for reference, but I'll get back to this later:
https://github.com/looker-open-source/look-at-me-sideways/tree/issue-32/__tests__/dummy-projects/07-issue-32

@tstodden
Copy link

👋🏼 I'm experiencing this false positive as well when I use the same parameter_name._parameter_value syntax in views. Upon initial investigation it seemed to be specific to dimension and not dimension_group.

@fabio-looker
Copy link
Contributor

Last time I looked at this, I wasn't able to correctly set up a failing test. This time I've taken a look at the F1 code and have an idea where the bug may be.. I won't be able to work on code changes until likely next week, so I'm commenting my findings for now:

The code that should handle ignoring _parameter_value is here: https://github.com/looker-open-source/look-at-me-sideways/blob/master/rules/f1.js#L64

However, the issue seems to be even before that, in the regex, which seems to be way too permissive in terms of what characters it captures (i.e. it will capture the spaces, equal signs, and remainder of the condition as though it were part of the reference)

https://github.com/looker-open-source/look-at-me-sideways/blob/master/rules/f1.js#L47

Also, I should do something to look for multiple liquid tags within the block. Currently, it doesn't seem to be a global match or otherwise loop to catch multiple instances of liquid.

@fabio-looker
Copy link
Contributor

Btw, @tstodden - In the short-term, if you're not yet adding an exemption to avoid this, I would recommend that as a temporary workaround. Thanks for the additional bug report!

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