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

[NETBEANS-1673]:Added support for auto-complete of enum case values i… #1059

Merged
merged 6 commits into from
Feb 14, 2019

Conversation

arusinha-zz
Copy link

@arusinha-zz arusinha-zz commented Dec 27, 2018

…n switch expression(jdk-12 feature)

Jira link : https://issues.apache.org/jira/browse/NETBEANS-1673

* @return the result of the invoked method.
* @since 2.41
*/
public static Object invokeMethod(String className, String methodName, List<Class> parameterTypes, Object obj, Object... paramArgs) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, but I don't think we should have a method like this in the API. That is not bringing any real value to the clients, and the use is (hopefully) transient anyway.

My recommendation is to isolate the reflection into the TreeShims, providing "high-level" methods (like getExpressions(Tree), enforcing that the Tree is a SwitchExpressionTree). With:
#1074

The only copy of TreeShims is in java.source.base, and the methods can be reused by java.hints and java.completion (and if needed by any other module).

Copy link
Author

Choose a reason for hiding this comment

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

Had removed ReflectionHelper in recent commit instead used TreeShims class

Copy link
Contributor

@jlahoda jlahoda left a comment

Choose a reason for hiding this comment

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

I don't think there should be an API change for this, and esp. not a method that only invokes reflection.

Also, tests appear to be missing?

@arusinha-zz arusinha-zz removed API Change [ci] enable extra API related tests work-in-progress labels Jan 21, 2019
@arusinha-zz
Copy link
Author

arusinha-zz commented Jan 21, 2019

I don't think there should be an API change for this, and esp. not a method that only invokes reflection.

Also, tests appear to be missing?

Had removed the API is recent commit instead TreeShims is used
Test Cases is added for the issue

Copy link
Contributor

@jlahoda jlahoda left a comment

Choose a reason for hiding this comment

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

Looks OK.

@arusinha-zz arusinha-zz merged commit d96f2d2 into apache:master Feb 14, 2019
@arusinha-zz arusinha-zz added the Cherry Pick Consider cherry picking for release update label Feb 19, 2019
@lkishalmi lkishalmi removed the Cherry Pick Consider cherry picking for release update label Feb 26, 2019
lkishalmi pushed a commit that referenced this pull request Feb 26, 2019
[NETBEANS-1673]:Added support for auto-complete of enum case values i…
@arusinha-zz arusinha-zz deleted the netbeans-1673 branch March 1, 2019 09:35
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.

3 participants