SONARPY-618 Introduce Ambiguous Symbol#663
Conversation
There was a problem hiding this comment.
I wonder if we could find a better name, e.g. alternatives()? I'm not sure it's really better.
There was a problem hiding this comment.
Can't we initialize this field in the constructor and remove this method?
There was a problem hiding this comment.
Shouldn't we call setSymbolsByDeclarationTree?
There was a problem hiding this comment.
I don't like changing symbols at this point. It seems very late. What are the risks that we end up with a graph of symbols which doesn't make sense? Aren't we changing symbols which are referenced by other symbols?
There was a problem hiding this comment.
I think we should find a way to do that differently, without needing AmbiguousSymbolImpl to store symbolsByDeclarationTree.
There was a problem hiding this comment.
One idea:
Add a symbol method to FunctionDefImpl. This symbol could be different from the symbol attached to the name of the function in the case of an ambiguous symbol.
There was a problem hiding this comment.
Good idea, I will go for it! Thanks!
16792c9 to
ed3e092
Compare
There was a problem hiding this comment.
I think that this condition is not sufficient. We end up creating an ambiguous symbol in the following case and I suppose we should not:
x = 42
x = 43
There was a problem hiding this comment.
Good point. I'll update the condition to also check the presence of a usage of kind FUNC_DECLARATION or CLASS_DECLARATION
74dda57 to
8a0fb17
Compare
8a0fb17 to
abb416f
Compare
|
Kudos, SonarQube Quality Gate passed! |
…buf (#663) GitOrigin-RevId: c922c1b9578d323b36e5f39c7801c8354e1976f7
No description provided.