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

Elect a renaming handler to check new references #124

Open
jvcoutinho opened this issue Dec 2, 2019 · 6 comments
Open

Elect a renaming handler to check new references #124

jvcoutinho opened this issue Dec 2, 2019 · 6 comments

Comments

@jvcoutinho
Copy link
Collaborator

jvcoutinho commented Dec 2, 2019

Currently, only one of the four renaming handlers check if a developer added a new reference to an edited method, and only under certain conditions. Although, another handler, NewElementReferencingEditedOne, checks if a developer added a new element that refers to an edited one, but again, under certain conditions.

So, as solution, we could do the following:

  • Elect one of the handlers to check new references. I vote for Merge Methods, once it's one of the simplest handlers and acts like an extension to semistructured merge by merging two nodes with different identifiers. Naturally, it has higher potential of reporting conflicts with this feature.
  • Implement this feature in all of the four renaming handlers.
  • Improve NewElementReferencingEditedOne handler to check this more reliably.
@pauloborba
Copy link
Collaborator

@jvcoutinho we have to first discuss how precise is the reference check. and how far it requires the project to be compilable? buildable?

@jvcoutinho
Copy link
Collaborator Author

jvcoutinho commented Feb 14, 2020

@pauloborba currently, it just checks at the same class level. This means new references to public methods in other classes aren't detected.
We can expand this to the whole project (makes more sense) or to the package level. But in both scenarios, this would be a feature that would only make sense if S3M runs as the git driver, because you can plug files without packages in the standalone version. And, of course, the whole project/package needs to be compilable.

@pauloborba
Copy link
Collaborator

@jvcoutinho does the current (class scoped reference check) implementation require the class to be syntactic valid? compilable?

@guilhermejccavalcanti given this class scope restriction, would you expect much difference from this reference check (in relation to the other handlers that do no checking)

@jvcoutinho
Copy link
Collaborator Author

@jvcoutinho does the current (class scoped reference check) implementation require the class to be syntactic valid? compilable?

Yes. We compile the class in order to catch the references.

@guilhermejccavalcanti
Copy link
Owner

@guilhermejccavalcanti given this class scope restriction, would you expect much difference from this reference check (in relation to the other handlers that do no checking)

I don't think so, at least from the observations of the NewElementReferencingEditedOne handler, there are very few occurrences. I can't tell about the cases of renaming.

But the main limitation of the compilation scope is the fact that on real use (as a git merge driver), the tool has only access to the merged file, not the entire project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants