SONARPY-2324 Document missing Django views due to missing resolution of self members #2148
Conversation
hashicorp-vault-sonar-prod
bot
changed the title
guillaume-dequenne-sonarsource
requested a review
from thomas-serre-sonarsource
guillaume-dequenne-sonarsource
force-pushed
the
document-s3752-fn
branch
3 times, most recently
from
5a384c2 to
bca5c0c
Compare
| from django.urls import path | ||
|
|
||
| class SameClassView: | ||
| def chosen_view(self): # FN SONARPY-2322 | ||
| ... | ||
|
|
||
| def get_urlpatterns(self): | ||
| return [path("something", self.chosen_view, name="something")] |
There was a problem hiding this comment.
minor for clarity, I would put this test in another file called views_as_class_member or views_as_class_method, or something similar
There was a problem hiding this comment.
That makes sense! I added views_and_urls_same_file to also cover a basic case of view defined in the same file as a url.
|
|
||
| from django.urls import path | ||
|
|
||
| class SameClassView: |
There was a problem hiding this comment.
nit I'm not sure SameClassView is crystal clear.
| class SameClassView: | |
| class ClassWithViews: |
or something similar
| from django.urls import path | ||
|
|
||
| class SameClassView: | ||
| def chosen_view(self): # FN SONARPY-2322 |
There was a problem hiding this comment.
| def chosen_view(self): # FN SONARPY-2322 | |
| def view_method(self): # FN SONARPY-2322 |
| class SameClassView: | ||
| def chosen_view(self): # FN SONARPY-2322 | ||
| ... | ||
|
|
There was a problem hiding this comment.
extra mile to get even closer to the original issue from our analysis, I would add a method using a property decorator :
@property
def view_property(self): and ensure that we have also the FN.
There was a problem hiding this comment.
The rule already tests for various decorators. I deliberately removed the @property one so that it doesn't bring confusion as to the root cause of the FN.
There was a problem hiding this comment.
ok makes sense !
| class SameClassView: | ||
| def chosen_view(self): |
There was a problem hiding this comment.
| class SameClassView: | |
| def chosen_view(self): | |
| class ClassWithViews: | |
| def view_method(self): |
| ProjectLevelSymbolTable projectSymbolTable = new ProjectLevelSymbolTable(); | ||
| projectSymbolTable.addModule(parseWithoutSymbols(content), "my_package", pythonFile("mod.py")); | ||
| // SONARPY-2322: should be true | ||
| assertThat(projectSymbolTable.isDjangoView("my_package.mod.SameClassView.chosen_view")).isFalse(); |
There was a problem hiding this comment.
I ran this test on commit d955f10, before the merge of MMF-3945, and I confirm that this assertion failed, i.e. chosen_view is a Django view on this old commit.
guillaume-dequenne-sonarsource
force-pushed
the
document-s3752-fn
branch
from
bca5c0c to
b74aaac
Compare
guillaume-dequenne-sonarsource
force-pushed
the
document-s3752-fn
branch
from
b74aaac to
458246b
Compare
|
guillaume-dequenne-sonarsource
requested a review
from thomas-serre-sonarsource
SONARPY-2324