Skip to content

Commit

Permalink
Fix lint findings (#604)
Browse files Browse the repository at this point in the history
* Address lint findings for osv/request_helper_test.py

osv/request_helper_test.py:31:2: C0116: Missing function or method docstring (missing-function-docstring)
osv/request_helper_test.py:33:4: C0103: Variable name "TEST_RESPONSE" doesn't conform to '^[a-z][a-z0-9_]*$' pattern (invalid-name)
osv/request_helper_test.py:36:4: C0103: Variable name "TEST_URL" doesn't conform to '^[a-z][a-z0-9_]*$' pattern (invalid-name)
osv/request_helper_test.py:16:0: W0611: Unused import os (unused-import) osv/request_helper_test.py:21:0: W0611: Unused import requests (unused-import)

* Address lint findings for request_helper.py

osv/request_helper.py:52:2: C0116: Missing function or method docstring (missing-function-docstring)

* Address lint findings for osv/ecosystems_test.py

osv/ecosystems_test.py:47:50: W0108: Lambda may not be necessary (unnecessary-lambda)
osv/ecosystems_test.py:142:23: W0621: Redefining name 'cache' from outer scope (line 22) (redefined-outer-name)

* Fix lint findings for osv/ecosystems.py

osv/ecosystems.py:120:0: C0301: Line too long (89/80) (line-too-long)
osv/ecosystems.py:354:0: C0301: Line too long (81/80) (line-too-long)
osv/ecosystems.py:362:6: R1705: Unnecessary "else" after "return" (no-else-return)
osv/ecosystems.py:405:2: R1705: Unnecessary "else" after "return" (no-else-return)

* Address spurious warnings about unclosed sockets

This makes the tests much quieter.

* Address lint findings in osv/debian_version_cache.py

osv/debian_version_cache.py:20:0: C0301: Line too long (110/80) (line-too-long)
osv/debian_version_cache.py:27:0: C0301: Line too long (90/80) (line-too-long)
osv/debian_version_cache.py:30:2: W0231: __init__ method from base class 'Exception' is not called (super-init-not-called)

* Partially address lint findings for osv/cache.py

osv/cache.py:53:23: R1735: Consider using {} instead of dict() (use-dict-literal)
osv/cache.py:60:4: R1705: Unnecessary "else" after "return" (no-else-return)

These ones remain at this time due to cognitive burden to address:

osv/cache.py:86:14: W1505: Using deprecated method getcallargs() (deprecated-method)
osv/cache.py:70:0: C0103: Function name "Cached" doesn't conform to the `snake_case` group in the '^(?:(?P<exempt>setUp|tearDown|setUpModule|tearDownModule)|(?P<camel_case>_?[A-Z][a-zA-Z0-9]*)|(?P<snake_case>_?[a-z][a-z0-9_]*))$' pattern (invalid-name)

* Address lint findings for gcp/appengine/frontend_handlers_test.py

gcp/appengine/frontend_handlers_test.py:19:0: W0611: Unused mock imported from unittest (unused-import)

* Address lint findings for gcp/api/test_server.py

gcp/api/test_server.py:100:2: W1510: Using subprocess.run without explicitly set `check` is not recommended. (subprocess-run-check)

In this instance, the stopping of the container is best-effort and
failing is fine.

* Address lint findings in gcp/api/server.py

gcp/api/server.py:92:4: W0621: Redefining name 'futures' from outer scope (line 17) (redefined-outer-name)

* Address lint findings in gcp/api/integration_tests.py

gcp/api/integration_tests.py:100:2: C0116: Missing function or method docstring (missing-function-docstring)

* Address lint findings for docker/worker/worker.py

docker/worker/worker.py:260:0: C0301: Line too long (89/80) (line-too-long)

* Partially address lint findings for docker/importer/importer.py

docker/importer/importer.py:262:8: W0612: Unused variable 'vulnerability' (unused-variable)

This remains due to cognitive load to refactor:

docker/importer/importer.py:213:4: R1702: Too many nested blocks (6/5) (too-many-nested-blocks)

* Address additional lint line-length findings overlooked

* Make decorator name style-guide compliant

* Adjust import for rename made in f80bf4e

* Refactor to not use deprecated inspect.getcallargs()

Credit to Rex Pan for helping with the refactoring.

#599 (comment)

* Refactor _process_updates_git() to eliminate lint finding

docker/importer/importer.py:213:4: R1702: Too many nested blocks (6/5) (too-many-nested-blocks)

* Promote _sync_from_previous_commit() to a top-level method

Per reviewer feedback:
8f7caf5#r946322047

* Remove invalid options

Despite evidence to the contrary, these options do not seem to be
supported (any more).

pylint-dev/pylint#6931 (comment)

* Use a generator instead

Eliminate lint finding:

gcp/appengine/frontend_handlers.py:381:25: R1728: Consider using a generator instead 'sum(len(entry.get('package', {}).get('name', '')) for entry in affected)' (consider-using-generator)

* Suppress arguments-differ lint finding for _pre_put_hook

osv/models.py:335:2: W0221: Number of parameters was 1 in 'Model._pre_put_hook' and is now 1 in overridden 'Bug._pre_put_hook' method (arguments-differ)
osv/models.py:699:2: W0221: Number of parameters was 1 in 'Model._pre_put_hook' and is now 1 in overridden 'SourceRepository._pre_put_hook' method (arguments-differ)

My running theory is that because these are decorated as @classmethod in
the abstract class (but don't specify cls as the first parameter), this
lint finding is getting triggered. I do not see a good way forward other
than suppression.

* Disable unnecessary-lambda-assignment lint finding

Hopefully only temporarily.

* Refactor away unnecessary lambda

With thanks to @another-rex
  • Loading branch information
andrewpollock authored Aug 17, 2022
1 parent f660fa6 commit 1726203
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 10 deletions.
6 changes: 0 additions & 6 deletions .pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,6 @@ good-names=main,_
# Bad variable names which should always be refused, separated by a comma.
bad-names=

# List of builtins function names that should not be used, separated by a comma.
bad-functions=input,apply,reduce


[FORMAT]

Expand All @@ -115,9 +112,6 @@ max-module-lines=99999
# String used as indentation unit. We differ from PEP8's normal 4 spaces.
indent-string=' '

# Make sure : in dicts and trailing commas are checked for whitespace.
no-space-check=


[TYPECHECK]

4 changes: 3 additions & 1 deletion gcp/api/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,9 @@ def do_query(query, context, include_details=True):
context.abort(grpc.StatusCode.INVALID_ARGUMENT, 'Invalid Package URL.')
return None

to_response = lambda b: bug_to_response(b, include_details)
def to_response(b):
return bug_to_response(b, include_details)

next_page_token = None

if query.WhichOneof('param') == 'commit':
Expand Down
2 changes: 1 addition & 1 deletion gcp/appengine/frontend_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ def event_value(event):
def should_collapse(affected):
"""Whether if we should collapse the package tab bar."""
total_package_length = sum(
[len(entry.get('package', {}).get('name', '')) for entry in affected])
len(entry.get('package', {}).get('name', '')) for entry in affected)
return total_package_length > 70 or len(affected) > 5


Expand Down
4 changes: 2 additions & 2 deletions osv/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ def _tokenize(self, value):
value_lower = value.lower()
return re.split(r'\W+', value_lower) + [value_lower]

def _pre_put_hook(self):
def _pre_put_hook(self): # pylint: disable=arguments-differ
"""Pre-put hook for populating search indices."""
search_indices = set()

Expand Down Expand Up @@ -696,7 +696,7 @@ def ignore_file(self, file_path):

return False

def _pre_put_hook(self):
def _pre_put_hook(self): # pylint: disable=arguments-differ
"""Pre-put hook for validation."""
if self.type == SourceRepositoryType.BUCKET and self.editable:
raise ValueError('BUCKET SourceRepository cannot be editable.')
Expand Down

0 comments on commit 1726203

Please sign in to comment.