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

CodeQL fixes and other misc. cleanups #168

Merged
merged 15 commits into from Nov 8, 2020

Conversation

FLHerne
Copy link
Contributor

@FLHerne FLHerne commented Nov 4, 2020

No description provided.

This was missed in the recent commit a1d23b6:
"Codechange: Various non-formatting related flake8 fixes"
There will only be an error here if the value isn't an integer.
The only values with defined meanings are 0 or 1, so in this case
 there's an error in the source and we should fail compilation.
This doesn't fix some circular-import issues that will need more
 refactoring to avoid.
Resolves all issues from the `flake8-comprehensions` plugin.
This conforms with the PEP 8 recommendations, and also sorts them
 alphabetically because it's opinionated. :p

This is the result of running `isort` and then `black`,
 unfortunately they fight with each other about formatting...
The uninitialized-variable checker doesn't understand the control flow
 well enough (in one case it's very implicit, the others are obvious).
@FLHerne FLHerne changed the title WIP: Various CodeQL fixes CodeQL fixes and other misc. cleanups Nov 8, 2020
This resoolves the CodeQL warning:
 Assignment overwrites attribute comment, which was previously
  defined in superclass.
 Subclasses should not set attributes that are set in the superclass.
 Doing so may violate invariants in the superclass.
This might also squash a false-positive CodeQL warning, not sure yet.
This might fix some CodeQL false-positives.
@FLHerne
Copy link
Contributor Author

FLHerne commented Nov 8, 2020

Note: the three "failing" CodeQL errors are the ones that I added suppression comments to, because of this bizarre design choice:
github/codeql#4511

Copy link
Member

@LordAro LordAro left a comment

Choose a reason for hiding this comment

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

LGTM

@FLHerne FLHerne merged commit a6400be into OpenTTD:master Nov 8, 2020
@FLHerne FLHerne deleted the flh-codeql-fixes branch November 8, 2020 20:02
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.

None yet

3 participants