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

Make flake8-bandit work with latest bandit 1.7.3 version #22

Merged
merged 1 commit into from
Mar 8, 2022

Conversation

nastra
Copy link
Contributor

@nastra nastra commented Feb 28, 2022

Fixes #21

flake8-bandit 1.7.3 (PyCQA/bandit#496)
introduced an fdata argument.

@nastra nastra force-pushed the fix-issue-with-latest-bandit branch from 3aaa001 to c991d91 Compare February 28, 2022 07:53
@sathieu
Copy link
Contributor

sathieu commented Feb 28, 2022

I created #23 keeping compatibility with bandit <= 1.7.2.

@Dreamsorcerer
Copy link

Maybe update the requirements on this PR to "bandit>=1.7.3" in setup.py? Then the maintainer can choose which PR to use, depending on whether they want to maintain backwards compatibility or not.

@nastra nastra force-pushed the fix-issue-with-latest-bandit branch from c991d91 to 7583561 Compare March 1, 2022 05:51
flake8_bandit.py Outdated
@@ -108,6 +108,7 @@ def _check_source(self):

bnv = BanditNodeVisitor(
self.filename,
None,
Copy link

@GriceTurrble GriceTurrble Mar 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest naming the arguments explicitly, as is being done in #23 :

bnv = BanditNodeVisitor(
    fname=self.filename,
    fdata=None,
    metaast=BanditMetaAst(),
    testset=BanditTestSet(BanditConfig(), profile=config.profile),
    debug=False,
    nosec_lines=[],
    metrics=Metrics(),
)

It costs nothing, makes the call easier to read, and highlights the real root cause of issues in future when/if Bandit changes something again.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, updated the code

@GriceTurrble
Copy link

GriceTurrble commented Mar 1, 2022

I'll copy here as well, I think there needs to be two releases pushed:

  • A PATCH update, v2.1.3, that sets the version for bandit<1.7.3 in setup.py (with no other functional change compared to v2.1.2)
  • A MINOR update, v2.2.0, that implements the change on this PR and sets bandit>=1.7.3

@nastra nastra force-pushed the fix-issue-with-latest-bandit branch from 7583561 to 0153473 Compare March 1, 2022 14:22
setup.py Outdated Show resolved Hide resolved
Fixes tylerwince#21

flake8-bandit 1.7.3 (PyCQA/bandit#496)
introduced an `fdata` argument and this just passes a `None` to make
things work with the latest version of bandit.
@nastra nastra force-pushed the fix-issue-with-latest-bandit branch from 0153473 to dfba032 Compare March 1, 2022 14:54
@Raniz85
Copy link

Raniz85 commented Mar 1, 2022

Another possibility would be to detect whether or not fdata is required by using the inspect module.

Something along the lines of:

kwargs = {}
if "fdata" in inspect.signature(BanditNodeVisitor).parameters:
  kwargs["fdata"] = None

bnv = BanditNodeVisitor(
    ....,
    **kwargs
)

Yet another solution would be to get bandit to make fdata default to None in 1.7.4 and then blacklist 1.7.3. This is probably the best solution imo since 1.7.3 introduced breaking changes in a bugfix release (if you follow semver at least).

Copy link

@carloscorrea93 carloscorrea93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool 🚀

Copy link

@staticdev staticdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@tylerwince tylerwince merged commit 16e6081 into tylerwince:master Mar 8, 2022
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

Successfully merging this pull request may close these issues.

Bandit 1.7.3 addition of new positional argument fdata causes TypeError
10 participants