Community
Participate
Working Groups
3.1 Unused import should not prevent compilation of the class. I get 'unresolved compilation problem'. That sounds too radical. There's nothing that prevents compiling the class.
Adam, Would you have an example? The builds contain unused imports and they don't prevent the class from being compiled.
ha, but i have the option set to error My point is that _even then_ it should not prevent the compiler from producing a fully usable classfile. It does produce a classfile but you can't really run anything in it. There's no _real_ problem with the code and I'm sure the compiler can notice that.
for an example try anything, like A.java import java.util.List;//error here public class A { public static void main(String[] args) { System.out.println("hello"); } } you get: Exception in thread "main" java.lang.Error: Unresolved compilation problem: at A.main(A.java:3)
ok, I see. This is much clearer. Thanks. So you mean we should filter out the warnings when generating problem types and problem methods. That sounds fair, but this means the warnings won't be fixed :-). Philippe, I will investigate this change.
>That sounds fair, but this means the warnings won't be fixed :-). I think of warnings as a compile-time thing. I once I press "run" and click "ok to run with errors in workspace" it means I do want to run. thanks Olivier. This kind of problem does not happen often but when it does you're really suprised that the compiler seems to buffled by a triviality like this.
We can clearly filter out the warnings, but errors will still result in problem methods.
Let me rephrase the title to reflect the real problem.
By 'errors' you mean only the 'Java Language Specification errors', right? And not 'JLS + whatever-preferences-you-have' errors. That's fair. If I have bogus code in my method then it's OK to let it fail when the method is called (OK to fail even on a branch than does not contain the problem, for simplicity).
By errors I mean whatever error is set in the compiler preferences. It is up to the user to decide what should be considered as an error. An error means that the code must be fixed. If I set unused import as error, then it is fair to get a runtime error even if this is not an error from the JLS point of view. I agree that warnings should cause runtime error to be thrown.
re comment 9 so how is my original problem going to be fixed? The class will still fail on runtime because of the unused import. I think this is incorrect and this is exactly why I entered the report. I think compilation should create problem methods in classfiled only on 'mandatory i.e., JLS errors'. On the optional (user specified) errors it should just go ahead and produce the bytecode as if nothing happened'.
Then this is not consistent with the user's settings. I thought that you had unused imports set as a warning. I misread your comment 2. I'll see what we can do with this.
>Then this is not consistent with the user's settings. That's OK with me. The mandatory (JLS) errors form a separate category anyway (you can't change their severity) so I think it's ok to distingush them here too.
The problem that we have right now is that the error prevents further analysis on the code. For example, the error can abort the type resolution of the unit and then no types in it have been resolved. We would need to tweak the whole error management in the compiler in order to change this behavior. Note that the batch compiler is not concerned with this since it can only consider unused imports as warnings and not as error. This would only impact the compiler within Eclipse. I will defer investigation till Philippe is back from vacations. Philippe, any comment?
Thanks Olivier.
Adam, What you want to do simply means that you should set the unused imports as a warning instead of an error. In this case, you could still see them in the problem view and you could run your code. Saying that I think that any error on an import statement should not prevent from generating the .class file. Especially for unused import. That import clearly doesn't do anything. So in this case we should not trigger the problem types/methods mode and simply generate the code. Now we could imagine a better problem view that could allow you to "sort" or "prioritize" the problems (warnings or errors) to give a higher priority to the one you really want to fix. Having all warnings at the same level can prevent from focusing on specific ones.
I noticed this problem in the following debugging scenario. During debugging I changed a bunch of code and built. Hotcode replace worked fine but I did not notice that an unused import got introduced. So now, when I tried calling a method on the class I got the unfriendly and not easily understandable 'sorry, I can't run this class because you have an unused import' error. It just looked too weird not to enter as a bug report :) Normally, I do want my unused imports as errors.
I admit that getting problem method/type for unused imports is not ideal. I will investigate what needs to be changed to get a better control of the kind of errors that should not generate problem types/methods.
Adam, I opened a more specific bug report for the unused imports case. See bug 107332.
This is the old ZRH request for having extra compiler errors not alter classfile generation. I am against allowing errors to produce valid classfiles. If you only want to get warned about an issue, then do not set it to ERROR, but WARNING. Errors are meant to be fatal. Introducing JLS error vs. optional error would be very misleading. Now, I understand some users are raising some warnings as errors to make them more proeminent, but I think this is an organizational problem. Why not fixing all warnings in the first place, then you can immediately see a new one. That's what we do in JDT/Core. If some warnings are not very interesting, and you cannot fix them, then simply disable them until you care again. This whole debate could be improved if we introduced a 3rd severity category (which the platform supports: INFO), where INFO < WARNING < ERROR.
> Errors are meant to be fatal. Introducing JLS error vs. optional error would be > very misleading. On the other hand, I would expect a Java compiler to create working class files as long as the JLS is not violated. Otherwise, it does not compile Java(TM) any more :-) > This whole debate could be improved if we introduced a 3rd severity category > (which the platform supports: INFO), where INFO < WARNING < ERROR. That's bug 47340.
I understand your claim Markus, but having fatal errors vs. ignorable errors would be hard to understand. How would you expect users to tell the difference ? Then you would start working in a mode where you could ignore some errors, which would easily make you miss real ones. Now the story is simple. Red X means must fix now, this is it. If you feel that unused imports are so bad, then simply fix them. I guess it takes less time than entering/annotating this defect...
I think there is a difference between compile-time errors and runtime errors. On compile time, I do want to see that import flagged as error. But as soon as I press Run and confirm I do want to run a project with errors (for whatever reason), I do want the bytecode to be as complete as possible. A triviality like an unused import should not prevent a compiler from generating valid code. This is especially painful in cases when you *don't* see the 'do you want to run with errors?' dialog - like in my original debug/hot swap scenario. See comment 16.
(In reply to comment #20) > On the other hand, I would expect a Java compiler to create working class files > as long as the JLS is not violated. Otherwise, it does not compile Java(TM) any > more :-) Markus, don't get confused between the compiler within Eclipse and the batch compiler. The batch compiler cannot report unused import as errors. It is only a warning. And also the whole idea of problem types/methods is specific to the Eclipse environment or the batch compiler with -processOnError option.
By default, our compiler is sticking to the JLS. You can make it more sensitive, but this has to be explicitly set by the user. You cannot blame the tool for rejecting what you told it to be an error. Basically, an error is just an error, which prevents running some portion of the code (an import spans over the entire unit, which makes it not ignorable). Based on your arguments, I don't get why you are setting unused imports as errors, given you want to ignore them. Maybe you should better select the problems you want to reveal, or ask the tooling to better provide means to categorize them.
>I don't get why you are setting unused imports as >errors, given you want to ignore them. I do not want to ingore them. But when by accident you introduce them, like in my debug/hot swap scenario (see comment 16), and the debugee fails because of an unresolved compile problem then you start questioning if it is the right thing to do to make the classfile unusable because of a triviality like an unused import. I really do not see why the set of problem (i.e. made unusable by the inserted throw-error bytecode) classes must be *exactly* the same set as the set of classes that have compile errors. It should be just the subset for which it is truly not possibly to generate code (like an unresolved reference, etc). Philippe, if you really feel strongly about this, it's ok for me. It's not a super huge deal for me. An not-understandible annoyance nonetheless.
No problem. I am trying to understand the exact scenario. I would like to keep the story simple. I feel concerned about introducing such an intermediate level of errors, which aren't truly errors.
Philippe, Proposed fix for bug 107814 gives some idea on how to filter such errors.
I saw it, but it looks a bit of a hack as filtering one by one on a per ID basis which would end up being very error prone. Also, I still do not believe this makes sense in the long term. How are we going to tell users that some errors are fine where others are fatal ? Our rule until now is simple and consistent: no error is fine, they are all fatal.
I believe related bug is rather bug 107332.
Also re: comment 22, I believe this is where we diverge. The compile time and runtime error sets are just the same. We let you run code with errors, but identify some damage areas based on error locations. An import spans over the entire unit, and thus is more involving than some error nested in some statement. But again, don't tell unused imports are errors if you do not want to cause damage to your classfiles. Again, why using ERROR for these problems ? Why isn't WARNING just fine ? My guess is that you have tons of warnings which you cannot/do not want to get rid off (since you accumulated so many), and then there are a few you want to make stand out. The problem is that these are interfering with some mechanisms as described here. My recommendation is that you simply fix all your warnings, then you can notice all unused import at the very moment you introduce it. Working with a low problem count is a good strategy, at least until we have better ways to categorize problems than the simple problem view. Also, I would rather favor introducing an INFO level, that you could use to downgrade your numerous and less important warnings, which are only making noise. Am I totally wrong here ?
Philippe: For me, your observation is correct except in one point. There are certain style warnings that we'd like to see flagged (as INFO?) but which are still OK to commit to CVS. Therefore, we don't want to take the effort to workaround/"fix" these warnings, but we still want to see them when editing the CU. Although I still think errors that don't prevent code generation should not lead to runtime errors, I'd be a happy camper if bug 47340 was implemented.
For these extra warnings, do you even care about them in the problem view ? I remember some old requests where some wanted more warnings in editor/reconcile than when building (i.e. do not care about certain warnings until you edit them).
I already filter warnings from the Problems view today. If info severity was available, I would just show errors and warnings in the Problems view and filter infos. It may be hard to understand for users why some problems only appear in the editor. And sometimes, I still want to see the infos in the problems view (e.g. to count occurrences of a certain problem to decide whether it should be promoted to warning or error). IMO, we should keep the story simple here, unless there are significant performance reasons for not creating markers for some problems.
Agreed 100%. I am simply trying to see what you had in mind exactly. <g>
Added compiler option so as to specify whether optional errors should be fatal or not. By default, an optional error is treated as fatal as a normal language error (as defined by the language spec book), when disabling this option, clients will be able to treat optional errors as severe warnings only, which will be rendered as errors, but no longer prevent from running the code. There is some work planned on UI side so as to better distinguish amongst mandatory vs. optional errors. <pre> * COMPILER / Treating Optional Error as Fatal * When enabled, optional errors (i.e. optional problems which severity is set to "error") will be treated as standard * compiler errors, yielding problem methods/types preventing from running offending code until the issue got resolved. * When disabled, optional errors are only considered as warnings, still carrying an error indication to make them more * severe. Note that by default, errors are fatal, whether they are optional or not. * - option id: "org.eclipse.jdt.core.compiler.problem.fatalOptionalError" * - possible values: { "enabled", "disabled" } * - default: "enabled" </pre>
Olivier - pls add some specific testcases. Note for UI: it shouldn't be surfaced as an option until the differenciation of marker/annotation is made in the UI.
To clarify: differenciation between mandatory and optional errors. Currently there is no API for it.
Marking as fixed. UI counterpart should have a PR for it in JDT/UI
Added regression tests in org.eclipse.jdt.core.tests.compiler.regression.NonFatalErrorTest.test001/005. This includes tests for bug 107332.
*** Bug 107332 has been marked as a duplicate of this bug. ***
I filed bug 113239 for the UI
Verified for 3.2 M3 using build I20051025-0800+JDT/Core v_618a bu modifying the jdt/core prefs in .settings.
*** Bug 168104 has been marked as a duplicate of this bug. ***
This is a great enhancement. Many teams want to flag unused imports, local variables, etc., as errors, but it has a negative effect on hot-swapping. My vote is to change the default of the new "Treat configurable errors like fatal" to false. Otherwise this improvement risks going undiscovered by most users.
