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

Variable called "java" messes with the imports #1320

Closed
orichalque opened this issue May 22, 2017 · 11 comments
Closed

Variable called "java" messes with the imports #1320

orichalque opened this issue May 22, 2017 · 11 comments
Labels

Comments

@orichalque
Copy link

Hello,
I'm Spooning the JUnit4 source code, and in the following class, a variable has been named "java":

String java = System.getProperty("java.home") + File.separator + "bin" + File.separator + "java";

I configured my launcher with launcher.getEnvironment().setAutoImports(false);, after running the launcher, the following code is generated:

java.lang.String java = ((((java.lang.System.getProperty("java.home")) + (java.io.File.separator)) + "bin") + (java.io.File.separator)) + "java";

Which is completely normal, but the compilation will fail because "java.lang.System" refers to the "java" String declared earlier, which has no "lang" attribute (obviously).

Since my previous issue I'm running Spoon using launcher.getEnvironment().setAutoImports(false);.

Since it is quite specific, I'm renaming the Java variable in the source code, but it can be a bit time-consuming, when there's multiple classes with a similar variable.

Thibault

@msteinbeck
Copy link
Contributor

Why is the package "java.lang" listed at all? Isn't it imported by default?

@orichalque
Copy link
Author

Hello @msteinbeck,
With AutoImports having the "false" value, Spoon specifies it. Is there a specific option for not importing the standard libraries such as String or System ?

@msteinbeck
Copy link
Contributor

Is there a specific option for not importing the standard libraries such as String or System ?

Actually, I can't answer this question but I assume that it is not possible. From my point of view, even when autoimports are disabled, java.lang should not be listed explicitly.

@surli
Copy link
Collaborator

surli commented May 22, 2017

Since my previous issue I'm running Spoon using launcher.getEnvironment().setAutoImports(false);.

You're issue has been fixed, so you should be able to use the autoimport with Spoon 5.7.0-SNAPSHOT. I'll release it soon.

java.lang.String java = ((((java.lang.System.getProperty("java.home")) + (java.io.File.separator)) + "bin") + (java.io.File.separator)) + "java";

This should not happen, so there's a real bug here: when autoImport is disabled we have a mechanism to check this kind of conflict and to import the minimum classes to avoid this kind of error.

Actually, I can't answer this question but I assume that it is not possible. From my point of view, even when autoimports are disabled, java.lang should not be listed explicitly.

From my point of view, there's already a lot of options in Spoon, it would be overwhelming to add another one :) However we can discuss about the contract of Spoon when autoImport is disabled.
In my opinion, the behaviour should remain to use fully-qualified name everywhere, when it's possible, so no exception for java.lang.

WDYT @monperrus @tdurieux @pvojtechovsky ?

@surli surli added the bug label May 22, 2017
@pvojtechovsky
Copy link
Collaborator

In my opinion, the behavior should remain to use fully-qualified name everywhere, when it's possible, so no exception for java.lang.

I agree with Simon. There is no reason to change behavior of spoon for implicit java packages. It is OK that spoon generates java.lang.String. I intuitively expect that

  • autoimport=true means use import + short class names when possible
  • autoimport=false means do not use import + qualified class names when possible

It is just my feeling. If somebody has good reason for another behavior, I have no problem with anything else too. No algorithm I know depends on that.

@monperrus
Copy link
Collaborator

monperrus commented May 22, 2017 via email

@msteinbeck
Copy link
Contributor

I didn't know that Spoon already fixes this issue with 5.7.0. That being said, I agree with you that importing java.lang in case of autoimport = false is suitable.

@surli
Copy link
Collaborator

surli commented May 23, 2017

I didn't know that Spoon already fixes this issue with 5.7.0

Just to clarify, when I said the bug is fixed to @orichalque I was talking about #1306 which may have led to the use of autoimport = false.

@surli
Copy link
Collaborator

surli commented May 23, 2017

Another question: do you all agree that in either mode, we never need to explicitly import anything from java.lang. So the output file will never contain an import java.lang.XXX, right?

@pvojtechovsky
Copy link
Collaborator

output file will never contain an import java.lang.XXX

I agree.

@msteinbeck
Copy link
Contributor

Another question: do you all agree that in either mode, we never need to explicitly import anything from java.lang. So the output file will never contain an import java.lang.XXX, right?

That is what i suggested above :)

surli added a commit to surli/spoon that referenced this issue May 23, 2017
surli added a commit to surli/spoon that referenced this issue May 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants