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

feat: read parameter names from bytecode if possible #4972

Merged
merged 2 commits into from
Jan 19, 2023

Conversation

danielbobbert
Copy link
Contributor

This feature allows Spoon to read the parameter name from Java 8 and later byte code that has been built with the "-parameters" flag such that CtParameter objects representing code from the class path contain the "real" parameter name instead of "argX".

@MartinWitt
Copy link
Collaborator

Hi,
First, thank you for the feature. In spoon we try to have a test for every feature. Could you try to add a test case for this feature? Please don't hesitate to ask for help if you need any.

@SirYwell
Copy link
Collaborator

I'm not sure how a test for this would look like. We either need .class files as resources (which is bad because we can't properly read them here) or compile a java file and read it then. For that, a proper test infrastructure that can be reused would be needed, otherwise it'll get ugly really quick.

@danielbobbert
Copy link
Contributor Author

danielbobbert commented Oct 27, 2022 via email

@I-Al-Istannen
Copy link
Collaborator

I-Al-Istannen commented Oct 28, 2022

I could potentially add a mvn sub-module to the project, which compiles a test class with the appropriate compiler settings for the sake of testing. I feels like quite a bit of overhead for this relatively small and straightforward addition.

I agree, that sounds a bit too heavy handed. What about an API like

	@Test
	void parameterNamesAreParsedWhenCompilingWithParametersFlag() throws ClassNotFoundException {
		ClassLoader loader = JavacFacade.compileFiles(
			Map.of(
				"Test.java",
				"class Test {\n"
					+ "  public void foo(String bar) {}\n" +
					"}\n"
			),
			List.of("-parameters")
		);
		CtType<?> test = new JavaReflectionTreeBuilder(createFactory()).scan(loader.loadClass("Test"));
		CtMethod<?> method = test.getMethodsByName("foo").get(0);
		CtParameter<?> parameter = method.getParameters().get(0);

		assertThat(parameter.getSimpleName(), is("bar"));
	}

Depending on what @MartinWitt and @SirYwell think, I could PR such a facade and then you can use this test (or one you come up with)? Alternatively we could leverage JDT to compile this for us with the parameter flag instead of javac. I'd assume javac is a bit more common though, but this shouldn't really matter.

EDIT: It could look something like this

@I-Al-Istannen
Copy link
Collaborator

@danielbobbert The facade was merged in master. If you rebase or merge this PR onto master, you can adapt the test case it brought with it and/or write an additional one :)

@I-Al-Istannen
Copy link
Collaborator

Hey @danielbobbert, is there anything we can help you with to finish this PR? :)

@I-Al-Istannen I-Al-Istannen force-pushed the feature/java_parameter_names branch from 4b14eee to 80cd1c0 Compare January 12, 2023 10:01
@I-Al-Istannen I-Al-Istannen changed the title feat: read parameter names from bytecode if possible review: feat: read parameter names from bytecode if possible Jan 12, 2023
Copy link
Collaborator

@SirYwell SirYwell left a comment

Choose a reason for hiding this comment

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

Looks good, will merge.

@SirYwell SirYwell changed the title review: feat: read parameter names from bytecode if possible feat: read parameter names from bytecode if possible Jan 19, 2023
@SirYwell SirYwell merged commit 914b113 into INRIA:master Jan 19, 2023
@monperrus monperrus mentioned this pull request Mar 9, 2023
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.

4 participants