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

Use instanceof-pattern #2357

Merged
merged 1 commit into from
Dec 9, 2022

Conversation

mickaelistria
Copy link
Contributor

No description provided.

@fbricon
Copy link
Contributor

fbricon commented Nov 29, 2022

Code doesn't compile:

16:18:35 [ERROR] Failed to execute goal org.eclipse.tycho:tycho-compiler-plugin:3.0.0:compile (default-compile) on project org.eclipse.jdt.ls.core: Compilation failure: Compilation failure:
16:18:35 [ERROR] /home/jenkins/agent/workspace/jdt-ls-pr/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corrections/proposals/GetterSetterCorrectionSubProcessor.java:[151]
16:18:35 [ERROR] if (!(coveringNode d SimpleName)) {
16:18:35 [ERROR] ^^^^^^^^^^^^
16:18:35 [ERROR] Syntax error on token "coveringNode", instanceof expected after this token
16:18:35 [ERROR] /home/jenkins/agent/workspace/jdt-ls-pr/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corext/refactoring/reorg/ReorgPolicyFactory.java:[4604]
16:18:35 [ERROR] log.markAsProcessed(JavaElementResourceMapping.create(javaElement);
16:18:35 [ERROR] ^
16:18:35 [ERROR] Syntax error, insert ")" to complete Expression
16:18:35 [ERROR] /home/jenkins/agent/workspace/jdt-ls-pr/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corext/refactoring/code/IntroduceParameterRefactoring.java:[287]
16:18:35 [ERROR] if (input instanceof ICompilationUnit cunit) {
16:18:35 [ERROR] ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
16:18:35 [ERROR] Expression type cannot be a subtype of the Pattern type
16:18:35 [ERROR] 3 problems (3 errors)

@mickaelistria
Copy link
Contributor Author

The 2 first ones are typos.
The later one is strangely not failing with JDT...

@mickaelistria mickaelistria force-pushed the instanceof-pattern branch 2 times, most recently from 8d9946d to 97001f0 Compare November 29, 2022 17:36
@snjeza
Copy link
Contributor

snjeza commented Dec 2, 2022

test this please

1 similar comment
@snjeza
Copy link
Contributor

snjeza commented Dec 2, 2022

test this please

@rgrunber
Copy link
Contributor

rgrunber commented Dec 6, 2022

I think this should be fine. I would just make sure to scan the entries for anything odd.

Copy link
Contributor

@rgrunber rgrunber left a comment

Choose a reason for hiding this comment

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

I think it looks pretty good. Haven't gone through all of it though :P

Just one small suggestion. I would avoid modifying anything under "org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corext" for now. Those are classes that were copied directly from JDT and I'm guessing these refactorings haven't happened in JDT repo yet.

Whenever we move some class from jdt.ui into jdt.core.manipulation, it's good to compare, we compare it with ours before deleting our copy to see if we made any changes that are different. It's easier to do if the classes aren't refactored in only one repo.

Exclude org.eclipse.jdt.ls.core.internal.corext
@mickaelistria
Copy link
Contributor Author

Just one small suggestion. I would avoid modifying anything under "org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corext" for now

OK, I reverted those.

@rgrunber rgrunber added this to the Early January 2023 milestone Dec 9, 2022
@rgrunber rgrunber merged commit 438e619 into eclipse-jdtls:master Dec 9, 2022
@mickaelistria
Copy link
Contributor Author

Thanks for merging!

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.

None yet

4 participants