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

Code Cleanup: Java 16 instanceof pattern matching #556

Conversation

SougandhS
Copy link
Contributor

@SougandhS SougandhS commented Nov 7, 2024

Performs code cleanup in pattern matching using instanceof operator according to Java 16 guidelines, making the code more concise for classes in jdt.launching package.

What it does

How to test

Author checklist

@SougandhS
Copy link
Contributor Author

Hi @jukzi , could you please check on this ?

@jukzi
Copy link
Contributor

jukzi commented Nov 7, 2024

Was this an pure automated cleanup, or manual edit? Or both?

@SougandhS
Copy link
Contributor Author

Was this an pure automated cleanup, or manual edit? Or both?

Manual edit only.

@@ -144,17 +144,17 @@ public class JavaLaunchableTester extends PropertyTester {
*/
private IType getType(IJavaElement element) {
IType type = null;
if (element instanceof ICompilationUnit) {
type= ((ICompilationUnit) element).findPrimaryType();
if (element instanceof ICompilationUnit iCompilationUnit) {
Copy link
Contributor

Choose a reason for hiding this comment

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

on github the intendation looks wrong here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in local its ok
image

Copy link
Contributor

Choose a reason for hiding this comment

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

It may be an automated conversion of TABs to whitespace or other way around. Would be good if the intendation would be uniform within a method. Please try autoformat the whole affected method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do this.

@@ -3337,8 +3331,8 @@ private static void updateCompliance(IVMInstall vm) {
if (LaunchingPlugin.isVMLogging()) {
LaunchingPlugin.log("Compliance needs an update."); //$NON-NLS-1$
}
if (vm instanceof IVMInstall2) {
String javaVersion = ((IVMInstall2)vm).getJavaVersion();
if (vm instanceof IVMInstall2 ivmInstall) {
Copy link
Contributor

Choose a reason for hiding this comment

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

intendation looks wrong on github

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in local there's no modification any intendations.
image

Copy link
Contributor

Choose a reason for hiding this comment

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

Enable "show whitespace characters" and you'll most likely see a mixture of tabs and spaces and different tools display tabs as 2,4,8,... whitespaces.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Enable "show whitespace characters" and you'll most likely see a mixture of tabs and spaces and different tools display tabs as 2,4,8,... whitespaces.

Thanks

@jukzi
Copy link
Contributor

jukzi commented Nov 7, 2024

beside minor intendation looks ok for me

@jukzi
Copy link
Contributor

jukzi commented Nov 7, 2024

Manual edit only.

please make that clear in the commit message "manual code cleanup" to not be confused with dogfooding

@SougandhS SougandhS force-pushed the Code_upgradation_instanceof_pattern_matching branch from 0a514b6 to 66814be Compare November 7, 2024 08:53
@SougandhS SougandhS requested a review from jukzi November 7, 2024 08:55
@SougandhS
Copy link
Contributor Author

Manual edit only.

please make that clear in the commit message "manual code cleanup" to not be confused with dogfooding

Done the changes now.

@SougandhS SougandhS force-pushed the Code_upgradation_instanceof_pattern_matching branch 4 times, most recently from 9f896cb to 7ae5a62 Compare November 7, 2024 09:43
@SougandhS
Copy link
Contributor Author

Hi @jukzi
I have removed all changes from JavaLaunchableTester.java as it is doing some auto formatting once it is saved, resulting huge collapse in indentations

* As well, this class provides VM install change notification,
* and computes class paths and source lookup paths for launch
* configurations.
* The central access point for launching support. This class manages the registered VM types contributed through the
Copy link
Contributor

Choose a reason for hiding this comment

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

Manu unrelated format changes in JavaRuntime.java

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same case as of JavaLaunchableTester.java - So removed the changes now.

@@ -119,7 +119,7 @@ protected ISourceContainer[] createSourceContainers() throws CoreException {
IPath path = entry.getPath();
IResource resource = root.findMember(path);
if (resource instanceof IContainer) {
Copy link
Contributor

Choose a reason for hiding this comment

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

missing cleanup here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@SougandhS SougandhS force-pushed the Code_upgradation_instanceof_pattern_matching branch from 7ae5a62 to 7a5de07 Compare November 7, 2024 10:47
Copy link
Contributor

@jukzi jukzi left a comment

Choose a reason for hiding this comment

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

LGTM, lets wait some days if someone else wants to review too.

@SougandhS
Copy link
Contributor Author

LGTM, lets wait some days if someone else wants to review too.

Sure @jukzi , Thanks

Performed manual code cleanup in pattern matching using
instanceof operator according to Java 16 guidelines,
making the code more concise for classes in jdt.launching package.
@jukzi jukzi force-pushed the Code_upgradation_instanceof_pattern_matching branch from 7a5de07 to a825216 Compare November 26, 2024 14:27
@eclipse-jdt-bot
Copy link
Contributor

This pull request changes some projects for the first time in this development cycle.
Therefore the following files need a version increment:

org.eclipse.jdt.launching/META-INF/MANIFEST.MF
org.eclipse.jdt.launching/pom.xml

An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch.

Git patch
From ff4d26cc976487c07d9b787175b39b58ff0034e1 Mon Sep 17 00:00:00 2001
From: Eclipse JDT Bot <jdt-bot@eclipse.org>
Date: Tue, 26 Nov 2024 14:29:35 +0000
Subject: [PATCH] Version bump(s) for 4.35 stream


diff --git a/org.eclipse.jdt.launching/META-INF/MANIFEST.MF b/org.eclipse.jdt.launching/META-INF/MANIFEST.MF
index 04685fba1..71435c3e1 100644
--- a/org.eclipse.jdt.launching/META-INF/MANIFEST.MF
+++ b/org.eclipse.jdt.launching/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.jdt.launching; singleton:=true
-Bundle-Version: 3.23.100.qualifier
+Bundle-Version: 3.23.200.qualifier
 Bundle-Activator: org.eclipse.jdt.internal.launching.LaunchingPlugin
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin
diff --git a/org.eclipse.jdt.launching/pom.xml b/org.eclipse.jdt.launching/pom.xml
index 4ddaac0a1..c92753988 100644
--- a/org.eclipse.jdt.launching/pom.xml
+++ b/org.eclipse.jdt.launching/pom.xml
@@ -18,7 +18,7 @@
   </parent>
   <groupId>org.eclipse.jdt</groupId>
   <artifactId>org.eclipse.jdt.launching</artifactId>
-  <version>3.23.100-SNAPSHOT</version>
+  <version>3.23.200-SNAPSHOT</version>
   <packaging>eclipse-plugin</packaging>
   
   <build>
-- 
2.47.0

Further information are available in Common Build Issues - Missing version increments.

@akurtakov
Copy link
Contributor

Thanks for this one. I really find such PRs reducing time to read code kind one of the most important things as with the limited resources it's very hard to dedicate time for issues/improvements making every second counts.

@akurtakov akurtakov merged commit ecfc91d into eclipse-jdt:master Nov 27, 2024
11 checks passed
@SougandhS
Copy link
Contributor Author

Thank you @akurtakov & @jukzi

@jukzi
Copy link
Contributor

jukzi commented Nov 27, 2024

I really find such PRs reducing time to read code kind one of the most important things as with the limited resources it's very hard to dedicate time for issues/improvements making every second counts.

I like such motivating comments. thanks. And thanks for @SougandhS for improving the code even in minor cases.

@SougandhS
Copy link
Contributor Author

I like such motivating comments. thanks. And thanks for @SougandhS for improving the code even in minor cases.

Thanks again @jukzi

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.

4 participants