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] py/unused-local-variable on SQLAlchemy model definition classes #11407

Open
amotl opened this issue Nov 24, 2022 · 4 comments
Open
Labels
acknowledged GitHub staff acknowledges this issue false-positive not security This issue does not relate to a security query Python

Comments

@amotl
Copy link

amotl commented Nov 24, 2022

Hi there,

thanks a stack for bringing LGTM to CodeQL. We used your kickstart template PR crate/crate-python#467 for making the transition happen on one of our Python repositories and wanted to report back about a potential false positive, after mitigating all other admonitions on our end before.

With kind regards,
Andreas.

Description of the false positive

py/unused-local-variable is raised on SQLAlchemy model definition classes, which are only defined, but not used.

Code samples or links to source code

class DummyTable(Base):
    __tablename__ = "t"
    pk = sa.Column(sa.String, primary_key=True)
    tags = sa.Column(ObjectArray)

Base.metadata.create_all()

URL to the alert on GitHub code scanning (optional)

Thoughts

I wonder if anything can be done about it, other than manually dismissing corresponding admonitions?

As far as we understand, CodeQL does not feature inline suppression comments/instructions, like what LGTM did with lgtm[py/import-and-import-from], right? (crate/crate-python@4397cc2e7)

Do you have any other suggestions on this matter?

@amotl amotl changed the title [False positive] SQLAlchemy model definition classes may yield py/unused-local-variable [False positive] py/unused-local-variable on SQLAlchemy model definition classes Nov 24, 2022
@MathiasVP
Copy link
Contributor

Indeed, this looks like a false positive. Thank you for reporting it!

Our current focus is on improving our security analysis. Because your report does not relate to a security query, we will put this on our backlog and prioritize it if we get enough reports of the same underlying issue in other projects.

As for the inline suppression comments, we're still discussing if Code Scanning should have a similar feature as LGTM. For now, the best solution is to dismiss the alerts manually.

@amotl
Copy link
Author

amotl commented Dec 2, 2022

Hi again,

we've dismissed the corresponding CodeQL notices about this issue, but apparently, they start re-appearing on subsequent pull requests, an example is crate/crate-python#474. Is this behavior intended?

With kind regards,
Andreas.

@amotl
Copy link
Author

amotl commented Dec 2, 2022

Oh I think because those are new occurrances, it is obvious that they would be reported. Sorry for the noise.

@amotl
Copy link
Author

amotl commented Dec 2, 2022

Just for the records. After we observed problems properly dismissing and resolving the admonition raised by @github-code-scanning bot, we reported them to github/codeql-action#1411.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
acknowledged GitHub staff acknowledges this issue false-positive not security This issue does not relate to a security query Python
Projects
None yet
Development

No branches or pull requests

2 participants