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

Incorrect edit script #165

Open
algomaster99 opened this issue Jul 9, 2021 · 9 comments
Open

Incorrect edit script #165

algomaster99 opened this issue Jul 9, 2021 · 9 comments

Comments

@algomaster99
Copy link
Member

left.java

class Invocation {
    int a = 1;
    void checkForSign() {
        if (a > 0) {
            System.out.println("y");
        } else if (a < 0) {
            System.out.println("z");
        } else {
            System.out.println("x");
        }
    }
}

right.java

class Invocation {
    int a = 1;
    void checkForSign() {
        if (a > 0) {
            System.out.println("x");
        } else if (a < 0) {
            System.out.println("y");
        } else {
            System.out.println("z");
        }
    }
}

EDIT SCRIPT

  1. Delete Block at Invocation:6
     {
     
     }
  2. Insert Block at Invocation:4
     {
     
     }
  3. Move Invocation from Invocation:9 to Invocation:5
     System.out.println("x")
  4.  Move Block from Invocation:4 to Invocation:6
     {
         System.out.println("y");
     }
  5.  Move Invocation from Invocation:7 to Invocation:9
     System.out.println("z")

Ideally, three move operations were enough but an optimum edit script is not guaranteed but a correct one should be. The edit script produced for this case has one major problem - the second operation inserts a block into an already existing block (if block is already there at 4). This could be a consequence of the absence of CtBlock concept in spoon but I am not sure. However, if this is the case, convertToSpoon needs to be fixed.

Using this edit script in diffmin also throws an error as elaborated here. This could be an issue with the mapping as THEN block in else-if in left.java should have been mapped to THEN block in if in right.java. However, GumTree matchers are based on heuristics so we cannot expect the matcher to give accurate mappings every time as it is an NP-hard problem as argued by @slarse.

tree

@algomaster99
Copy link
Member Author

@monperrus @khaes-kth srcDiff's output for this diff.
Screenshot from 2022-02-25 11-02-34

@monperrus
Copy link
Contributor

cool. and the Gumtree-JDT output?

@algomaster99
Copy link
Member Author

GumTree-JDT (master version as of the date this comment was created).
Screenshot from 2022-02-25 11-18-34
Screenshot from 2022-02-25 11-19-50

It is not really clear which element moved unless you have read the edit script.

@khaes-kth
Copy link
Contributor

@algomaster99 Is it fair to say they all fail on this diff?

@algomaster99
Copy link
Member Author

Mergely

https://editor.mergely.com/Bmo5md7x/

GitHub diff view

https://github.com/algomaster99/diffmin-examples/pull/7/files

We can claim that all of them fail, if we say the correct way to show the diff was 3 updates.

@monperrus
Copy link
Contributor

"Fail" may be too binary. We rather want an ordering relationship: X is better than Y.

Here, I'd say that scrDiff is better than gumtree-JDT for example.

And that the Github diff is the best.

@khaes-kth
Copy link
Contributor

Agreed. As always, there is no problem, only an "opportunity" for us to improve :)

@monperrus
Copy link
Contributor

monperrus commented Feb 25, 2022 via email

@algomaster99
Copy link
Member Author

Here, I'd say that scrDiff is better than gumtree-JDT for example.
And that the Github diff is the best.

I agree with your opinions.

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

No branches or pull requests

3 participants