-
Notifications
You must be signed in to change notification settings - Fork 30
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
Create rule S7156 #4491
base: master
Are you sure you want to change the base?
Create rule S7156 #4491
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
{ | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
{ | ||
"title": "\"copy.replace\" should not be invoked with an unsupported argument", | ||
"type": "CODE_SMELL", | ||
"status": "ready", | ||
"remediation": { | ||
"func": "Constant/Issue", | ||
"constantCost": "5min" | ||
}, | ||
"tags": [], | ||
"defaultSeverity": "Major", | ||
"ruleSpecification": "RSPEC-7156", | ||
"sqKey": "S7156", | ||
"scope": "All", | ||
"defaultQualityProfiles": ["Sonar way"], | ||
"quickfix": "unknown", | ||
"code": { | ||
"impacts": { | ||
"MAINTAINABILITY": "HIGH", | ||
"RELIABILITY": "MEDIUM" | ||
}, | ||
"attribute": "CONVENTIONAL" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
:object_replacement_protocol: https://docs.python.org/3/library/copy.html#object.__replace__ | ||
|
||
== Why is this an issue? | ||
|
||
Calling ``++copy.replace(...)++`` with an argument of an unsupported type will raise an ``++TypeError++``. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be a good idea to start by stating that this is for Python 3.13. i.e.: |
||
Types supported by ``++copy.replace(...)++`` must implement the {object_replacement_protocol}[replace protocol]. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is a good start, but you should write the rule thinking the person reading it sees |
||
|
||
The following built-in types are supported by ``++copy.replace(...)++`` | ||
|
||
* ``++collections.namedtuple()++`` | ||
* ``++dataclasses.dataclass++`` | ||
* ``++datetime.datetime++``, ``++datetime.date++``, ``++datetime.time++`` | ||
* ``++inspect.Signature++``, ``++inspect.Parameter++`` | ||
* ``++types.SimpleNamespace++`` | ||
* https://docs.python.org/3/reference/datamodel.html#code-objects[code objects] | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would just add a section about exceptions saying that this rule only raises for Python 3.13. |
||
== How to fix it | ||
|
||
If the argument passed in is a class defined in this project then implementing the {object_replacement_protocol}[replace protocol] by defining the ``++__replace__++`` method. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bear in mind that the section |
||
|
||
[source,python,diff-id=1,diff-type=compliant] | ||
---- | ||
class SomeClass: | ||
def __init__(self, name) | ||
self.name = name | ||
|
||
def __replace__(self, /, **changes) | ||
return SomeClass(changes.get("name", self.name)) | ||
---- | ||
|
||
=== Code examples | ||
|
||
==== Noncompliant code example | ||
|
||
[source,python,diff-id=1,diff-type=noncompliant] | ||
---- | ||
import copy | ||
|
||
class AClass: | ||
... | ||
|
||
a = AClass() | ||
b = copy.replace(a) # Noncompliant | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When commenting with |
||
---- | ||
|
||
==== Compliant solution | ||
|
||
[source,python,diff-id=1,diff-type=compliant] | ||
---- | ||
import copy | ||
|
||
class AClass: | ||
... | ||
def __replace__(self, /, **changes): | ||
... | ||
|
||
a = AClass() | ||
b = copy.replace(a) # Compliant | ||
|
||
|
||
@dataclass | ||
class ADataClass: | ||
... | ||
|
||
c = ADataClass() | ||
d = copy.replace(c) # Compliant | ||
---- | ||
|
||
=== Pitfalls | ||
|
||
Ensure that if the ``++__replace__++`` is implemented that the implementation creates a new object instead of updating the old one. | ||
|
||
|
||
== Resources | ||
=== Documentation | ||
* https://docs.python.org/3/library/copy.html#copy.replace | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can find how to format these sections and links in the docs folder . There are also a few guidelines available in the docs of sonarsource |
||
* https://docs.python.org/3/library/copy.html#object.\\__replace__ | ||
* https://docs.python.org/3/whatsnew/3.13.html#copy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually here we do give a small description. Something like
this rule raises an issue when copy.replace is used on an incorrect type.
It is optional but I think it is nice if we have rule that all have follow a similar structure.