Create rule S7156 #4491
Create rule S7156 #4491
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Seppli11
force-pushed
the
rule/add-RSPEC-S7156
branch
from
259bb2a to
5b7dc5e
Compare
Seppli11
force-pushed
the
rule/add-RSPEC-S7156
branch
from
5b7dc5e to
1a20440
Compare
| @@ -0,0 +1,78 @@ | |||
| :object_replacement_protocol: https://docs.python.org/3/library/copy.html#object.__replace__ | |||
|
|
|||
There was a problem hiding this comment.
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.
rules/S7156/python/rule.adoc
Outdated
| Calling ``++copy.replace(...)++`` with an argument of an unsupported type will raise an ``++TypeError++``. | ||
| Types supported by ``++copy.replace(...)++`` must implement the {object_replacement_protocol}[replace protocol]. |
There was a problem hiding this comment.
I think this is a good start, but you should write the rule thinking the person reading it sees .replace for the first time. Maybe glance a bit at the other rules, the numpy or pandas rules. Also you could see with the rules for Java they usually have a nicely detailed WHY.
rules/S7156/python/rule.adoc
Outdated
|
|
||
| == 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.
Bear in mind that the section How to fix and Why is it an issue are on different tab. So it is best to treat them as if the user did not read the previous section. So here for example it would be better to go with If the argument passed to the replace method.... Also in this section you can be more assertive. Instead of then implementing... you could go with to fix the issue, implement the protocol...
rules/S7156/python/rule.adoc
Outdated
| ... | ||
|
|
||
| a = AClass() | ||
| b = copy.replace(a) # Noncompliant |
There was a problem hiding this comment.
When commenting with # Noncompliant it would be good to also say why. So Noncompliant: the class A does not implement...
rules/S7156/python/rule.adoc
Outdated
|
|
||
| == Resources | ||
| === Documentation | ||
| * https://docs.python.org/3/library/copy.html#copy.replace |
There was a problem hiding this comment.
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
| * ``++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.
I would just add a section about exceptions saying that this rule only raises for Python 3.13.
rules/S7156/python/rule.adoc
Outdated
|
|
||
| == 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.
I think it would be a good idea to start by stating that this is for Python 3.13. i.e.: In Python 3.13 the copy.replace...
Seppli11
force-pushed
the
rule/add-RSPEC-S7156
branch
2 times, most recently
from
55577ac to
67f9f22
Compare
Seppli11
force-pushed
the
rule/add-RSPEC-S7156
branch
from
67f9f22 to
04f68e9
Compare
|
|
You can preview this rule here (updated a few minutes after each push).
Review
A dedicated reviewer checked the rule description successfully for: