SpoonLabs / gumtree-spoon-ast-diff

Computes the AST difference (aka edit script) between two Spoon Java source code abstract syntax trees
http://hal.archives-ouvertes.fr/hal-01054552
Apache License 2.0
161 stars 42 forks source link

Some offsets seem to be problematic #327

Closed pouryafard75 closed 2 months ago

pouryafard75 commented 2 months ago

This is a continuation of the previously reported issue https://github.com/SpoonLabs/gumtree-spoon-ast-diff/issues/323. Thank you very much @algomaster99 @monperrus for the prompt fix.

For the following code snippet:

class Test {
    @Override
    public int hashCode() {
        return -1;
    }
}

I get the following tree:

root [0,0]
    RootPac: unnamed package [0,0]
        Class: Test [0,80]
            Method: hashCode [17,78]
                RETURN_TYPE: int [0,0]
                Modifiers_Method [17,78]
                    Modifier: public [31,36]
                Annotation: java.lang.Override [17,25]
                Return: return [63,72]
                    UnaryOperator: NEG [70,71]
                        Literal: 1 [71,71]

The problems that you can address here:

algomaster99 commented 2 months ago

Hi @pouryafard75 !

I am unable to reproduce the error. I get:

root [0,0]
    RootPac: unnamed package [0,0]
        Class: Test [0,80]
            Method: hashCode [17,78]
                RETURN_TYPE: int [38,40]
                Modifiers_Method [31,36]
                    Modifier: public [31,36]
                Annotation: java.lang.Override [17,25]
                Return: return [63,72]
                    UnaryOperator: NEG [70,71]
                        Literal: 1 [71,71]

RETURN_TYPE is [38,40].

Literal has the same start and offset (71). Modifer public has 36 and 31 while public takes 6 characters!

This is because of https://github.com/INRIA/spoon/issues/5980#issuecomment-2366855384. @SirYwell, could you recommend APIs that would give the correct lengths. Right now, we could add 1 to the current result and get the correct length.

SirYwell commented 2 months ago

@SirYwell, could you recommend APIs that would give the correct lengths. Right now, we could add 1 to the current result and get the correct length.

I don't think there is a direct API for that, we only provide start and end position, both inclusive. Calculating the distance between them and adding 1 sounds like the correct solution for the length, and we could probably add a method for that to spoon.

algomaster99 commented 2 months ago

Thanks!

pouryafard75 commented 2 months ago

@algomaster99 Could you please run the AstComperator with the following arguments? src/test/resources/examples/annotations/left.java src/test/resources/examples/annotations/right.java

and print the tree as:

final Diff result = new AstComparator().compare(new File(args[0]), new File(args[1]));
System.out.println(result.getMappingsComp().src.toTreeString());

This is what I get:

root [0,0]
    RootPac: unnamed package [0,0]
        Class: Test [0,80]
            Method: hashCode [17,78]
                RETURN_TYPE: int [0,0]
                Modifiers_Method [17,78]
                    Modifier: public [31,36]
                Annotation: java.lang.Override [17,25]
                Return: return [63,72]
                    UnaryOperator: NEG [70,71]
                        Literal: 1 [71,71]
algomaster99 commented 2 months ago

I still get this as output:

root [0,0]
    RootPac: unnamed package [0,0]
        Class: Test [0,80]
            Method: hashCode [17,78]
                RETURN_TYPE: int [38,40]
                Modifiers_Method [31,36]
                    Modifier: public [31,36]
                Annotation: java.lang.Override [17,25]
                Return: return [63,72]
                    UnaryOperator: NEG [70,71]
                        Literal: 1 [71,71]

This is very strange.

I also have a CI test ensuring this behaviour. RETURN_TYPE must be enclosed within hashCode so its start position must be >=17 and end position must be <=78. And here is the specific test case.

I can only recommend using the latest release.

pouryafard75 commented 2 months ago

Sorry my bad, I was experimenting with https://github.com/SpoonLabs/gumtree-spoon-ast-diff/pull/325/commits/28af6e1613ead5b4af63bb0d583220d9a94d4647 since you had mentioned me back then. I also get

RETURN_TYPE: int [38,40], however since the end offsets are off by 1, I can't test it properly.

algomaster99 commented 2 months ago

Sorry for not being clear initially 😅

however since the end offsets are off by 1, I can't test it properly.

Good news is that all are offset by 1 so you can always add 1 to the end position. If you cannot do this, then you can add an API to https://github.com/inria/spoon like SirYwell suggests by first opening an issue there.

pouryafard75 commented 2 months ago

@algomaster99 I give it a try, Thanks

algomaster99 commented 2 months ago

Closing. Feel free to open another issue in case you encounter some problems :)