Community
Participate
Working Groups
-------------------------------------------------------- class Diamond { void foo() { Map<String, String> m = new HashMap<>(); } public static void main(String[] args) { new Diamond().foo(); } } -------------------------------------------------------- - Select "new HashMap<>()" - Refactor > Introduce Parameter => ------------------------------------------------------- class Diamond { void foo(HashMap<String, String> hashMap) { Map<String, String> m = hashMap; } public static void main(String[] args) { new Diamond().foo(new HashMap<>()); //error here } } -------------------------------------------------------
Ill have a look.
Created attachment 199099 [details] Patch + tests The error was because the ParameterInfo for the diamond was set from the expression as it is without inferring the parameters. I have fixed it in the UI code although there can be APIs from core on Expression to determine if it has a diamond and whether the type parameters can be inferred. Markus, could you pls have a look at the fix?
- You need to use the import rewrite to create the type references. Never create the type nodes yourself from a binding. Example that fails (introduce parameter for "new java.util.HashMap<>()"): import java.util.Map; public class Diamond { class HashMap {} void foo() { Map<String, ? extends Number> m = new java.util.HashMap<>(); } public static void main(String[] args) { new Diamond().foo(); } } Don't create objects/variables unless you're sure you use them later ('ast' and 'simpleName' go inside the "if (parameters.length > 0)" block). - classInstanceCreation.toString(): Javadoc says this is only for debugging purposes. Use ASTNodes#asFormattedString(..) for now. Eventually, this refactoring should use ChangeSignatureProcessor#setDefaultValueAdvisor(..), but that would be too much for now. We have other open bugs for the missing rewriting of values.
To check whether we really deal with a diamond, use ClassInstanceCreation#getType(), test instanceof ParameterizedType and then check typeArguments() == 0. See bug 349517 for the API request ClassInstanceCreation#isResolvedTypeInferredFromExpectedType(), but i don't think we need that here.
Ping!
(In reply to comment #4) > See bug 349517 for the API request > ClassInstanceCreation#isResolvedTypeInferredFromExpectedType(), but i don't > think we need that here. That is bug 350897 :)
Created attachment 200502 [details] Patch + tests Patch creates type using ImportRewriteContext so that the qualifier is not removed (replaced with an import) for cases shown in comment #3. Markus, could you pls review?
The "if (parameters.length > 0)" is a no-op (always true), since parameterized type bindings always have type arguments. See comment 4 for the right test. This can e.g. be seen when you have comments in the existing actual type arguments. classInstanceCreation.setType(..) should not be called like this, since this modifies the original AST. However, this is the easiest solution and the whole thing is a hack anyway, so I left it in but restored the original AST node again. The tests failed because the results were wrongly formatted. Fixed with commit 2ffd9ab835396cfd7d6c2ece7e8e4297a15c25d5.
Tests were even more screwed than I initially saw: - not formatted, whitespace garbage - bad class names that lead to conflicting updates in unrelated test classes, depending on execution order Furthermore, IntroduceParameterTests#testSimple_Capture() also failed, which apparently neither of us ever ran. Bug was that you used the result of typeBinding.getName() as type name, which is wrong (fails in this case and doesn't add imports). Fixed everything with commit ab5a5917c6c3b36cd87f4bb10f197dd67d229257.
> Fixed everything with commit [..]. Got a push conflict, it's commit 317dc3d4384797a7c39d20d5060c363099cf125d.
