Skip to content

Commit

Permalink
Create cleanup actions for some existing quick assists.
Browse files Browse the repository at this point in the history
- Add final modifier where possible
- Convert switch statement to switch expression
- Use pattern matching for instanceof checks
- Convert anonymous function to lambda expression
- Add test cases, and set CleanUpsTest compliance to 19
- Due to LSP limitations (no overlapping text edits), we now return the
  final state of the document after all cleanups are applied as a text
  edit

Signed-off-by: Roland Grunberg <[email protected]>
  • Loading branch information
rgrunber committed Nov 30, 2022
1 parent e7e1418 commit 1e87984
Show file tree
Hide file tree
Showing 7 changed files with 496 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,11 @@
import java.util.Map;

import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.jdt.core.IBuffer;
import org.eclipse.jdt.core.ICompilationUnit;
import org.eclipse.jdt.core.IJavaProject;
import org.eclipse.jdt.core.JavaCore;
import org.eclipse.jdt.core.JavaModelException;
import org.eclipse.jdt.core.manipulation.CleanUpContextCore;
import org.eclipse.jdt.ls.core.internal.JDTUtils;
import org.eclipse.lsp4j.TextDocumentIdentifier;
Expand All @@ -42,6 +45,10 @@ public CleanUpRegistry() {
cleanUpsList.add(new AddDeprecatedAnnotationCleanUp());
cleanUpsList.add(new StringConcatToTextBlockCleanUp());
cleanUpsList.add(new InvertEqualsCleanUp());
cleanUpsList.add(new VariableDeclarationFixCleanup());
cleanUpsList.add(new SwitchExpressionCleanup());
cleanUpsList.add(new InstanceofPatternMatch());
cleanUpsList.add(new LambdaExpressionCleanup());

// Store in a Map so that they can be accessed by ID quickly
cleanUps = new HashMap<>();
Expand Down Expand Up @@ -94,10 +101,28 @@ public List<TextEdit> getEditsForAllActiveCleanUps(TextDocumentIdentifier textDo

// build the context after setting the compiler options so that the built AST has all the required markers
CleanUpContextCore context = CleanUpUtils.getCleanUpContext(textDocumentId, opts, monitor);

List<TextEdit> textEdits = new ArrayList<>();
for (ISimpleCleanUp cleanUp : cleanUpsToRun) {
textEdits.addAll(CleanUpUtils.getTextEditFromCleanUp(cleanUp, context, monitor));
ICompilationUnit cu = context.getCompilationUnit();

try {
ICompilationUnit wc = cu.getWorkingCopy(monitor);
for (ISimpleCleanUp cleanUp : cleanUpsToRun) {
org.eclipse.text.edits.TextEdit jdtEdit = CleanUpUtils.getTextEditFromCleanUp(cleanUp, context, monitor);
if (jdtEdit != null) {
wc.applyTextEdit(jdtEdit, monitor);
context = CleanUpUtils.getCleanUpContext(wc, opts, monitor);
}
}
// https://microsoft.github.io/language-server-protocol/specifications/specification-3-16/#textEditArray
// Cleanups may have overlapping text edits but LSP does not support this
// Generate text edit as the entire document
IBuffer wcBuff = wc.getBuffer();
String newText = wcBuff.getContents();
TextEdit te = new TextEdit(JDTUtils.toRange(cu, 0, cu.getBuffer().getLength()), newText);
textEdits.add(te);

} catch (JavaModelException e) {
// continue
}

return textEdits;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
*******************************************************************************/
package org.eclipse.jdt.ls.core.internal.cleanup;

import java.util.Collections;
import java.util.List;
import java.util.Map;

import org.eclipse.core.runtime.CoreException;
Expand All @@ -27,9 +25,8 @@
import org.eclipse.jdt.internal.corext.dom.IASTSharedValues;
import org.eclipse.jdt.ls.core.internal.JDTUtils;
import org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin;
import org.eclipse.jdt.ls.core.internal.TextEditConverter;
import org.eclipse.lsp4j.TextDocumentIdentifier;
import org.eclipse.lsp4j.TextEdit;
import org.eclipse.text.edits.TextEdit;

/**
* Functions for working with JDT ICleanUpCore and ISimpleCleanUp.
Expand All @@ -53,6 +50,11 @@ public static CleanUpContextCore getCleanUpContext(TextDocumentIdentifier textDo
return new CleanUpContextCore(unit, ast);
}

public static CleanUpContextCore getCleanUpContext(ICompilationUnit unit, Map<String, String> compilerOpts, IProgressMonitor monitor) {
CompilationUnit ast = createASTWithOpts(unit, compilerOpts, monitor);
return new CleanUpContextCore(unit, ast);
}

/**
* Returns a non-null list of text edits for the given clean up.
*
Expand All @@ -64,25 +66,20 @@ public static CleanUpContextCore getCleanUpContext(TextDocumentIdentifier textDo
* the progress monitor
* @return a non-null list of text edits for the given clean up
*/
public static List<TextEdit> getTextEditFromCleanUp(ISimpleCleanUp cleanUp, CleanUpContextCore context, IProgressMonitor monitor) {
public static TextEdit getTextEditFromCleanUp(ISimpleCleanUp cleanUp, CleanUpContextCore context, IProgressMonitor monitor) {

try {
ICleanUpFixCore fix = cleanUp != null ? cleanUp.createFix(context) : null;
if (fix == null) {
return Collections.emptyList();
return null;
}
CompilationUnitChange cleanUpChange = fix.createChange(monitor);
org.eclipse.text.edits.TextEdit jdtEdit = cleanUpChange.getEdit();
if (jdtEdit == null) {
return Collections.emptyList();
}
TextEditConverter converter = new TextEditConverter(context.getCompilationUnit(), jdtEdit);
List<TextEdit> lspEdit = converter.convert();
return lspEdit != null ? lspEdit : Collections.emptyList();
return jdtEdit;
} catch (CoreException e) {
JavaLanguageServerPlugin.logError(String.format("Failed to create text edit for clean up %s", cleanUp.getIdentifier()));
}
return Collections.emptyList();
return null;
}

private static CompilationUnit createASTWithOpts(ICompilationUnit cu, Map<String, String> opts, IProgressMonitor monitor) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*******************************************************************************
* Copyright (c) 2022 Red Hat Inc. and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-2.0
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Red Hat Inc. - initial API and implementation
*******************************************************************************/
package org.eclipse.jdt.ls.core.internal.cleanup;

import java.util.Collections;
import java.util.List;

import org.eclipse.core.runtime.CoreException;
import org.eclipse.jdt.core.dom.CompilationUnit;
import org.eclipse.jdt.core.manipulation.CleanUpContextCore;
import org.eclipse.jdt.core.manipulation.ICleanUpFixCore;
import org.eclipse.jdt.internal.corext.fix.PatternMatchingForInstanceofFixCore;

/**
* Represents a cleanup that uses pattern matching for 'instanceof' cast checks
*/
public class InstanceofPatternMatch implements ISimpleCleanUp {

/* (non-Javadoc)
* @see org.eclipse.jdt.ls.core.internal.cleanup.ISimpleCleanUp#getIdentifier()
*/
@Override
public String getIdentifier() {
return "instanceofPatternMatch";
}

/* (non-Javadoc)
* @see org.eclipse.jdt.ls.core.internal.cleanup.ISimpleCleanUp#createFix(org.eclipse.jdt.core.manipulation.CleanUpContextCore)
*/
@Override
public ICleanUpFixCore createFix(CleanUpContextCore context) throws CoreException {
CompilationUnit unit = context.getAST();
if (unit == null) {
return null;
}
return PatternMatchingForInstanceofFixCore.createCleanUp(unit);
}

/* (non-Javadoc)
* @see org.eclipse.jdt.ls.core.internal.cleanup.ISimpleCleanUp#getRequiredCompilerMarkers()
*/
@Override
public List<String> getRequiredCompilerMarkers() {
return Collections.emptyList();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*******************************************************************************
* Copyright (c) 2022 Red Hat Inc. and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-2.0
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Red Hat Inc. - initial API and implementation
*******************************************************************************/
package org.eclipse.jdt.ls.core.internal.cleanup;

import java.util.Collections;
import java.util.List;

import org.eclipse.core.runtime.CoreException;
import org.eclipse.jdt.core.dom.CompilationUnit;
import org.eclipse.jdt.core.manipulation.CleanUpContextCore;
import org.eclipse.jdt.core.manipulation.ICleanUpFixCore;
import org.eclipse.jdt.internal.corext.fix.LambdaExpressionsFixCore;

public class LambdaExpressionCleanup implements ISimpleCleanUp {

/* (non-Javadoc)
* @see org.eclipse.jdt.ls.core.internal.cleanup.ISimpleCleanUp#getIdentifier()
*/
@Override
public String getIdentifier() {
return "lambdaExpression";
}

/* (non-Javadoc)
* @see org.eclipse.jdt.ls.core.internal.cleanup.ISimpleCleanUp#createFix(org.eclipse.jdt.core.manipulation.CleanUpContextCore)
*/
@Override
public ICleanUpFixCore createFix(CleanUpContextCore context) throws CoreException {
CompilationUnit unit = context.getAST();
if (unit == null) {
return null;
}
return LambdaExpressionsFixCore.createCleanUp(unit, true, false);
}

/* (non-Javadoc)
* @see org.eclipse.jdt.ls.core.internal.cleanup.ISimpleCleanUp#getRequiredCompilerMarkers()
*/
@Override
public List<String> getRequiredCompilerMarkers() {
return Collections.emptyList();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*******************************************************************************
* Copyright (c) 2022 Red Hat Inc. and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-2.0
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Red Hat Inc. - initial API and implementation
*******************************************************************************/
package org.eclipse.jdt.ls.core.internal.cleanup;

import java.util.Collections;
import java.util.List;

import org.eclipse.core.runtime.CoreException;
import org.eclipse.jdt.core.dom.CompilationUnit;
import org.eclipse.jdt.core.manipulation.CleanUpContextCore;
import org.eclipse.jdt.core.manipulation.ICleanUpFixCore;
import org.eclipse.jdt.internal.corext.fix.SwitchExpressionsFixCore;

/**
* Represents a cleanup that converts a switch statement to a switch expression
*/
public class SwitchExpressionCleanup implements ISimpleCleanUp {

/* (non-Javadoc)
* @see org.eclipse.jdt.ls.core.internal.cleanup.ISimpleCleanUp#getIdentifier()
*/
@Override
public String getIdentifier() {
return "switchExpression";
}

/* (non-Javadoc)
* @see org.eclipse.jdt.ls.core.internal.cleanup.ISimpleCleanUp#createFix(org.eclipse.jdt.core.manipulation.CleanUpContextCore)
*/
@Override
public ICleanUpFixCore createFix(CleanUpContextCore context) throws CoreException {
CompilationUnit unit = context.getAST();
if (unit == null) {
return null;
}
return SwitchExpressionsFixCore.createCleanUp(unit);
}

/* (non-Javadoc)
* @see org.eclipse.jdt.ls.core.internal.cleanup.ISimpleCleanUp#getRequiredCompilerMarkers()
*/
@Override
public List<String> getRequiredCompilerMarkers() {
return Collections.emptyList();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*******************************************************************************
* Copyright (c) 2022 Red Hat Inc. and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-2.0
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Red Hat Inc. - initial API and implementation
*******************************************************************************/
package org.eclipse.jdt.ls.core.internal.cleanup;

import java.util.Collections;
import java.util.List;

import org.eclipse.core.runtime.CoreException;
import org.eclipse.jdt.core.dom.CompilationUnit;
import org.eclipse.jdt.core.manipulation.CleanUpContextCore;
import org.eclipse.jdt.core.manipulation.ICleanUpFixCore;
import org.eclipse.jdt.internal.corext.fix.VariableDeclarationFixCore;

/**
* Represents a cleanup that adds the 'final' modifier where possible
*/
public class VariableDeclarationFixCleanup implements ISimpleCleanUp {

/* (non-Javadoc)
* @see org.eclipse.jdt.ls.core.internal.cleanup.ISimpleCleanUp#getIdentifier()
*/
@Override
public String getIdentifier() {
return "addFinalModifier";
}

/* (non-Javadoc)
* @see org.eclipse.jdt.ls.core.internal.cleanup.ISimpleCleanUp#createFix(org.eclipse.jdt.core.manipulation.CleanUpContextCore)
*/
@Override
public ICleanUpFixCore createFix(CleanUpContextCore context) throws CoreException {
CompilationUnit unit = context.getAST();
if (unit == null) {
return null;
}
return VariableDeclarationFixCore.createCleanUp(unit, true, true, true);
}

/* (non-Javadoc)
* @see org.eclipse.jdt.ls.core.internal.cleanup.ISimpleCleanUp#getRequiredCompilerMarkers()
*/
@Override
public List<String> getRequiredCompilerMarkers() {
return Collections.emptyList();
}

}
Loading

0 comments on commit 1e87984

Please sign in to comment.