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

Propose a fix to #1033 and add a new test for TemplateMatcher #1034

Merged
merged 11 commits into from
Dec 9, 2016
Merged

Propose a fix to #1033 and add a new test for TemplateMatcher #1034

merged 11 commits into from
Dec 9, 2016

Conversation

surli
Copy link
Collaborator

@surli surli commented Dec 9, 2016

Fix #1033 by looking if the TemplateMatcher has been detected in the find and remove it if it the case. Moreover this PR add a new dynamic contract by checking before doing any scan, if the TemplateMatcher is able to match itself.

However I still wonder why the find method in TemplateMatcher does return a List and not a Set or at least a Collection ?

monperrus and others added 10 commits November 16, 2016 22:28
…ith an or instead of an and. Now the test is failing, proof that the assertion is checked :)
… a local variable in the same block or by a field (or a variable in another block). First detection implemented but patch not satisfying.
…Show another bug with templateMatcher matching itself.
…ate matcher must find itself to prove it runs correctly.

scanner.scan(templateRoot);
if (!finds.contains(templateRoot)) {
throw new SpoonException("TemplateMatcher was unable to find itself, it certainly express a problem somewhere in the template.");
Copy link
Collaborator

Choose a reason for hiding this comment

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

TemplateMatcher was unable to find itself, it certainly indicates a bug. Please revise your template or report an issue.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll change that.

@monperrus
Copy link
Collaborator

However I still wonder why the find method in TemplateMatcher does return a List and not a Set or at least a Collection ?

what do you mean?

@monperrus
Copy link
Collaborator

so is there a bug behind #1025 ?

@surli
Copy link
Collaborator Author

surli commented Dec 9, 2016

However I still wonder why the find method in TemplateMatcher does return a List and not a Set or at least a Collection ?

what do you mean?

The signature of the method find in the TemplateMatcher is the following:
public List<CtElement> find(final CtElement targetRoot)

But I don't really see the point of returning a List: is it possible to have multiple times the same element in the findings? Or should each finding be an independent element (as in a Set)?

so is there a bug behind #1025 ?

Frankly I don't know: I tried to show the bug by taking elements from the stackoverflow question to create a test but apparently it works fine: I ask the guy to have a look on my test and to try the last Spoon version to check if it's ok for him...
Maybe we can close the issue and reopen it or create a new one depending on his answer?

@monperrus monperrus merged commit e7ab86e into INRIA:master Dec 9, 2016
@monperrus
Copy link
Collaborator

Maybe we can close the issue and reopen it or create a new one depending on his answer?

good idea

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

Successfully merging this pull request may close these issues.

2 participants