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

FastenUriUtils parsing incorrectly the "class", "method" for some cases with lambda function. #260

Open
cg122 opened this issue Mar 26, 2021 · 4 comments
Assignees

Comments

@cg122
Copy link
Contributor

cg122 commented Mar 26, 2021

Describe the bug

A clear and concise description of what the bug is.

To Reproduce

Parsing the partial uri in the following FASTEN uri

fasten://mvn!nl.tudelft.jpacman:jpacman-framework$8.1.1/nl.tudelft.jpacman.ui/PacManUiBuilder$addStopButton%28Lnl$tudelft$jpacman$game$Game%3A%29V%3A30$Lambda.$newInstance(%2Fnl.tudelft.jpacman.game%2FGame)PacManUiBuilder$addStopButton%28Lnl$tudelft$jpacman$game$Game%3A%29V%3A30$Lambda

output:

[nl.tudelft.jpacman.ui, PacManUiBuilder$addStopButton(Lnl$tudelft$jpacman$game$Game:)V:30$Lambda, ui/PacManUiBuilder$addStopButton, Lnl$tudelft$jpacman$game$Game:)V:30$Lambda.$newInstance(/nl.tudelft.jpacman.game/Game)PacManUiBuilder$addStopButton(Lnl$tudelft$jpacman$game$Game:, V:30$Lambda.$newInstance(/nl.tudelft.jpacman.game/Game)PacManUiBuilder$addStopButton(Lnl$tudelft$jpacman$game$Game:)V:30$Lambda]

Expected behavior

class name should be "PacManUiBuilder", method should be "addStopButton"

Additional context

Result got on develop branch (1001e75)

@MrLightful
Copy link
Member

MrLightful commented Apr 23, 2021

@cg122 I adjusted implementation towards the better way. It returns PacManUiBuilder as the class, but newInstance as the method. However, it is more of an "ideological problem" which of two to return (newInstance or addStopButton).

@ashkboos will contact you to know some context behind it. Do you think there might be different scenarios when either of them are needed? In this case, we need to solve this issue somehow? Maybe a nested array of method names in the order of inclusion? For example, ['addStopButton', 'newInstance']?

@cg122 also, do you maybe have artifacts similar to this one, with inner classes? Just to test the implementation.

And also, sorry for the late fix 😅

@cg122
Copy link
Contributor Author

cg122 commented Apr 23, 2021

Thanks @romatallinn. I will look at it and see how it works for other example.

@cg122
Copy link
Contributor Author

cg122 commented Apr 26, 2021

I checked the source code corresponding to the FASTEN uri above:

    private void addStopButton(final Game game) {
        assert game != null;

        buttons.put(STOP_CAPTION, game::stop);
    }

I guess the uri corresponds to 'Game::stop', and the simplified form of passing method reference used here complicates the understanding of the uri.

@proksch
Copy link
Contributor

proksch commented Mar 10, 2022

I just stumbled across this issue... as there are no follow-up comments, I would assume that the problem still persists. To me this looks more like a problem of how the FastenUri is created, rather than how it is parsed.

Who in the team was involved in the creation of the FastenUri part? (Sorry, that I do not know that, but it was before my time :D)

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

4 participants