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

minor code smells in static code analysis #1676

Closed
zacharyburnett opened this issue Nov 2, 2023 · 4 comments
Closed

minor code smells in static code analysis #1676

zacharyburnett opened this issue Nov 2, 2023 · 4 comments
Labels

Comments

@zacharyburnett
Copy link
Member

when running SonarQube scanning on asdf in the bench repository, it found some minor code smells:

duplicates in regex character class

https://plsonarqube.stsci.edu/project/issues?tags=regex&branch=sonarscan&id=spacetelescope-bench&open=AYttO5qgrmNdxaPHqW54

delimiters = r"[\W\- ]+"

variable name shadows builtin

https://plsonarqube.stsci.edu/project/issues?tags=pitfall&branch=sonarscan&id=spacetelescope-bench


cmp = "greater than"

type = {"type": [{"minimum": 3}]}

buffer = io.BytesIO()

buffer = reader.read()

buffer = np.empty((data_size,), np.uint8)

buffer = self._buffer

buffer = self._buffer

buffer = self._buffer[:size]

@braingram
Copy link
Contributor

Do these cause any issue for the benchmarking?

Looking over the "smells" I see:

  • several in the copied/vendorized jsonschema
  • uses of the word 'buffer'. I guess this is too generic of a word for sonarqubes liking?

@zacharyburnett
Copy link
Member Author

zacharyburnett commented Nov 3, 2023

Do these cause any issue for the benchmarking?

Looking over the "smells" I see:

  • several in the copied/vendorized jsonschema
  • uses of the word 'buffer'. I guess this is too generic of a word for sonarqubes liking?

There weren't any issues with benchmarking; it had a bunch of even more minor (minor-er?) suggestions that I ignored, but in this case it said cmp, type, and buffer are apparently already built-in functions / object names, so that might cause insidious errors

@braingram
Copy link
Contributor

If these aren't an issue for the benchmarking I'm inclined to leave the code as-is. The cmp and type uses are within the vendorized jsonschema which will hopefully be replaced with a dependency if jsonschema re-adds the features required by asdf.

As far as I'm aware,buffer is a builtin only in python 2, perhaps sonarqube isn't scanning for only python 3 issues?

@zacharyburnett
Copy link
Member Author

ok, sounds good to me then! I'll mark the issues as "ignore" in sonarqube

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants