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

Feature request: a rule for detecting calls to mutate all-caps "constants" #423

Open
drew-gill opened this issue Nov 22, 2023 · 1 comment

Comments

@drew-gill
Copy link

drew-gill commented Nov 22, 2023

In various languages, ALL_CAPS variable names are conventionally used to denote constants. Pre-python 3.8's final property, there is no standard solution for denoting constants, so in some instances the developers' intentions are only conveyed through this naming convention.

However, due to the lack of enforcement of this constant property pre-python 3.8, a class of bugs can occur when mutable objects are used for these "constants", such that mutating functions can be used without warning. The following shows just one such bug possible in this situation:

>>> TEST = {"entry1": [1]} # represents a "CONSTANT"
>>> my_channels1 = TEST.get("entry1", []) # get entry1 reference
>>> print(my_channels1)
[1]
>>> my_channels1.append(2) # adds a channel to the TEST value
>>> my_channels2 = TEST.get("entry1", []) # grabs the same reference
>>> print(my_channels2)
[1, 2]
>>> print(TEST) # TEST was modified! This is the bug we want to catch
{'entry1': [1, 2]}
>>> TEST2 = {"entry1": [1]} # Let's try again
>>> my_channels2 = TEST2.get("entry1", [])
>>> print(my_channels2)
[1]
>>> my_channels = list(set(my_channels2 + [2]))  # Let's make a new local variable
>>> print(my_channels)
[1, 2]
>>> print(TEST2) # TEST2 was NOT modified
{'entry1': [1]}

We'd like to warn developers that if they intend to create a constant, identified by this ALL_CAPS naming convention, they should make the value immutable (e.g. tuple, frozenset, MappingProxyType, Final). This can teach developers to use these tools at the right time (but is not a substitute for onboarding/docs material, as this convention is easy to not adhere to).

Specification proposal

If:

  • a variable is named in all-capital letters (excluding symbols/numbers)
  • AND the variable is assigned to a mutable object type

Then the new rule should:

  • raise an error, suggesting that that the object type should be immutable OR the variable should be renamed to not all-caps
@cooperlees
Copy link
Collaborator

I like this. Nudging people to use correct immutable types where appropriate is a great side effect. I will accept a PR to do this.

Please try and accurately suggest the immutable type that could possibly work if possible for bonus points.

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

No branches or pull requests

2 participants