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

Enhancements to Selector Extractor #7660

Merged
merged 14 commits into from Nov 5, 2020

Conversation

guillep
Copy link
Member

@guillep guillep commented Nov 2, 2020

This PR enhances the selector extractor heuristics for the new faulty error nodes (that introduced some bugs). The main idea of this PR is to fix those bugs and go beyond what was supported already in Pharo8:

  • add tests to handle different scenarios (selection or no selection, broken code...) and better support evolutions of this
  • extract the functionality to make it independent from Rubric, so Rubric can benefit from it but also the new spec2 tools.

This PR makes several enhancements to the heuristics already working in Pharo8.

  • we can now search senders and implementors in return nodes
  • the new heuristics try to get the selectors that are closer to the cursor (e.g., foo| bar was returning bar in P8 instead of foo)
  • ambiguous cursor locations (e.g., foo:|bar) give priority to the preceeding symbols as we thought it would be less surprising. Also, ambiguity can be just removed by properly formatting the code.
  • selections in cascades are disambiguated between the cascade receiver and the messages (read the class comments)
  • selections in blocks work properly

To make this, we also had to improve the parser to properly insert error nodes where expected (e.g, messages without receivers, closing parenthesis/brackets without corresponding openers, invalid cascade sequences)

Copy link
Member

@MarcusDenker MarcusDenker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I downloaded the image and tested it a bit: Works!

  • there is still one user of findSelectorFromAST, I guess that can use the same heuristics later
  • I wonder if having a dedicated version of #bestNodeFor: just for finding the selector would not allow to move some of the heuristics there.

@guillep
Copy link
Member Author

guillep commented Nov 5, 2020

I downloaded the image and tested it a bit: Works!

  • there is still one user of findSelectorFromAST, I guess that can use the same heuristics later

Yes, I've cleaned up a bit, but this is not the only one to clean. There is also findClassFromAST, findSelectorFromAST,#findNextKeywordIn:selection:searchingForward:ifFound:, #findValidKeywordIn:at:ifFound:...

  • I wonder if having a dedicated version of #bestNodeFor: just for finding the selector would not allow to move some of the heuristics there.

I don't think so. I'd keep the AST as clean as possible. To me the best we can do form the AST point of view is to enhance further the parser to better detect syntactic errors and make #bestNodeFor: robust enough to account for those :)

@MarcusDenker MarcusDenker merged commit 3f758fb into pharo-project:Pharo9.0 Nov 5, 2020
@guillep guillep deleted the senders-implementors branch December 12, 2023 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants