Community
Participate
Working Groups
Consider the following code: --------------------------------------------- import java.util.stream.Stream; public class Test { void x() { s = Stream.of(1, 2, 3) .limit(2); } } --------------------------------------------- Calling the quick fix "create local variable 's'" on 's' produces the following, undesired output --------------------------------------------- public class Test { void x() { Stream<Integer> s = Stream.of(1, 2, 3) .limit(2); } } --------------------------------------------- Expected output: --------------------------------------------- public class Test { void x() { Stream<Integer> s = Stream.of(1, 2, 3) .limit(2); } } --------------------------------------------- The quick fix should be unopinionated about my formatting style.
Mateusz, what can we do here? I believe quick fix always goes through rewrite, which means the formatting will change. But what's interesting is that the output changes when I do a format.
(In reply to Jay Arthanareeswaran from comment #1) > Mateusz, what can we do here? I believe quick fix always goes through > rewrite, which means the formatting will change. Yeah, the formatter only gets this line to format: Stream<Integer> s=MISSING(); So nothing can be done at this level, something in ASTRewrite would need to change. > But what's interesting is that the output changes when I do a format. You mean the "Expected output" from comment 0? It depends on formatter wrapping settings.
Lukas, can you check your formatter wrapper settings? If the output after the rewrite is as per your preference, then I am afraid we can't do much here.
Hmm, I don't see how formatting settings are related to this issue. I mean a) other options don't have this effect, including "create field" and "create parameter" b) It works differently (but still incorrectly) like this: --------------------------------------------- import java.util.stream.Stream; public class Test { void x() { s = /* */ Stream.of(1, 2, 3) .limit(2); } } --------------------------------------------- which produces --------------------------------------------- import java.util.stream.Stream; public class Test { void x() { Object s = /* */ Stream.of(1, 2, 3) .limit(2); } } ---------------------------------------------
(In reply to Lukas Eder from comment #4) > a) other options don't have this effect, including "create field" and > "create parameter" In those cases (after the rewrite) and in the code specified as expected output in comment #0, can you try what happens when you format the code? If the output changes, then you could fix the behavior by adjusting your formatter settings. Mateusz, BTW, I played around with the Assignment's wrapper options but can't quite get the format specified as expected output. Any hints? (In reply to Mateusz Matela from comment #2) > Yeah, the formatter only gets this line to format: > Stream<Integer> s=MISSING(); > So nothing can be done at this level, something in ASTRewrite would need to > change. This needs to be looked at.
(In reply to Jay Arthanareeswaran from comment #5) > In those cases (after the rewrite) and in the code specified as expected > output in comment #0, can you try what happens when you format the code? The output does change. > If the output changes, then you could fix the behavior by adjusting your > formatter settings. While that might be a workaround (in case I do find which one of the hundreds of formatting options gets involved here), I still don't fully understand why the formatter is applied in the first place. I really wish the formatter were applied only explicitly, when I want it to be applied. In the case of Stream formatting (or any builder pattern API), I *never* want the formatter to be applied. I understand you don't agree with this, but if there were at least an option somewhere in the preferences where I can say: "Leave formatting untouched after any refactoring action", or an option "Format only when explicitly requested", then I'd be very happy. :-)
(In reply to Lukas Eder from comment #6) > I really wish > the formatter were applied only explicitly, when I want it to be applied. > > In the case of Stream formatting (or any builder pattern API), I *never* > want the formatter to be applied. > > I understand you don't agree with this, but if there were at least an option > somewhere in the preferences where I can say: "Leave formatting untouched > after any refactoring action", or an option "Format only when explicitly > requested", then I'd be very happy. :-) <rant> to leave formatting untouched seems to conflict with the requirement to change the code. It has to be changed in some format, doesn't it? What about this "unformatted" result: public class Test { void x() { Stream<Integer> s = Stream.of(1, 2, 3) .limit(2); } } is that better? For other quick fixes it would be even worse, think of quick fix to implement an inherited abstract method, should it really be created "without format"? @Override public String test() throws IOException { // TODO Auto-generated method stub return null; } this doesn't even compile </rant> Just saying :)
(In reply to Stephan Herrmann from comment #7) > to leave formatting untouched seems to conflict with the requirement to > change the code. It has to be changed in some format, doesn't it? OK, you're right. By that definition, some "formatting" needs to be done. So, I'll bite > > What about this "unformatted" result: > [...] > is that better? No. "Obviously" not. But what's "obvious"? > For other quick fixes it would be even worse, think of quick fix to > implement an inherited abstract method, should it really be created "without > format"? I suspect that *in this case* it would be the expected default. But not in my case. All I'm asking Eclipse to do is amend the variable declaration with a type definition, or, fix the syntax. I have every reason to assume that this will only ever prepend the type to the variable name at the exact location where the variable was, and not format a subsection of my code that seems unrelated to the action itself. Conversely, check this out: ---------------------------------------- import java.util.stream.Stream; public class Test { void x() { // Deliberate excess indentation s = Stream.of(1, 2, 3) .limit(2); } } ---------------------------------------- Now, repeat the exercise to get this: ---------------------------------------- import java.util.stream.Stream; public class Test { void x() { Stream<Integer> s = Stream.of(1, 2, 3) .limit(2); } } ---------------------------------------- Somehow, the excess indentation seems to be more important than the fact that I like newlines in this particular case. Now, consider this: ---------------------------------------- import java.util.stream.Stream; public class Test { void x() { Stream<Integer> s1; s = s1 = Stream.of(1, 2, 3) .limit(2); } } ---------------------------------------- Repeat the exercise to get this (finally, the "wanted" behaviour): ---------------------------------------- import java.util.stream.Stream; public class Test { void x() { Stream<Integer> s1; Stream<Integer> s = s1 = Stream.of(1, 2, 3) .limit(2); } } ---------------------------------------- Apply formatting to get again this: ---------------------------------------- import java.util.stream.Stream; public class Test { void x() { Stream<Integer> s1; Stream<Integer> s = s1 = Stream.of(1, 2, 3) .limit(2); } } ---------------------------------------- I could do this all day to show you that this refactoring hardly ever matches my formatting rules, such that formatting currently has a different effect from refactoring. :) I don't think there's a single rule to rule them all, which is to always apply "some" formatting. You're right in your case. When overriding a method, the stub can be formatted according to some overriding stub formatting rules, if you want. But a general rule will never be consistent (probably), unless you format the entire file after refactoring, which really isn't close to what anyone wants. Ideally, you'll just implement the "minimal intrusion" (just prepend the type, don't really modify the rest) and let the user decide if they want to format any given section according to their rules. "Just saying" ;-) And I totally rest my case that I would like to be able to opt out of any implicit, automatic formatting from Eclipse.
I see two directions in this discussion: (1) Make rewriting smarter to further minimize format changes. (2) A global preference option to make rewriting on behalf of quick fixes totally format-agnostic. @Lukas, are you really asking for (2)? It sounds like it, but to me this is really hard to believe; anticipating that in the majority of cases you'll be disappointed how stupid format-agnostic rewriting is. Conversely, I believe we can all easily agree on the intention (1), but here my feeling is: much has already been done to apply formatting only were needed, and further fine tuning *may* be quite an effort compared to a small gain. At least the observation re "Stream<Integer> s=MISSING();" indicates some room for improvement in this particular case. So should we focus on this? (In reply to Lukas Eder from comment #8) > Ideally, you'll just implement the "minimal intrusion" (just prepend > the type, don't really modify the rest) ... here's where I think it might be technical issues, what gets in our way: There is no AST node where we can simply "prepend the type", we have to replace an Assignment by a SingleVariableDeclaration. I assume that this technicality currently dictates the scope of formatting. It'd be a bad excuse, for sure, just saying that this seemingly trivial request cannot be served by our current infra structure (if my interpretation is right).
(In reply to Stephan Herrmann from comment #9) > I see two directions in this discussion: > (1) Make rewriting smarter to further minimize format changes. > (2) A global preference option to make rewriting on behalf of quick fixes > totally format-agnostic. Excellent. > @Lukas, are you really asking for (2)? It sounds like it, but to me this is > really hard to believe; anticipating that in the majority of cases you'll be > disappointed how stupid format-agnostic rewriting is. I'm not really asking for this, but if the Ctrl+Shift+F rules should apply (partially) as suggested here, then I might review that opinion. To me Ctrl+Shift+F always formats a given and large scope, e.g. a selection. In this case, formatting is much more local, not even spanning an entire expression. > Conversely, I believe we can all easily agree on the intention (1), but here > my feeling is: much has already been done to apply formatting only were > needed, and further fine tuning *may* be quite an effort compared to a small > gain. > At least the observation re "Stream<Integer> s=MISSING();" indicates some > room for improvement in this particular case. So should we focus on this? Sure! :) When I opened this issue, I only wanted to focus on this particular case. Then, the formatting discussion somehow started and escalated...
(In reply to Jay Arthanareeswaran from comment #5) > Mateusz, BTW, I played around with the Assignment's wrapper options but > can't quite get the format specified as expected output. Any hints? You're right, it cannot be done. We'd need to introduce a new indentation policy - "Do not indent".
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie.
