Skip to content

Commit

Permalink
First draft for JDT clean-up.
Browse files Browse the repository at this point in the history
  • Loading branch information
fvgh committed Aug 27, 2019
1 parent 777d0b7 commit 2085159
Show file tree
Hide file tree
Showing 11 changed files with 695 additions and 159 deletions.
13 changes: 13 additions & 0 deletions _ext/eclipse-jdt/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,22 @@ ext {

dependencies {
compile "com.diffplug.spotless:spotless-eclipse-base:${VER_SPOTLESS_ECLISPE_BASE}"
/*
* JDT core manipulation required for clean-up base interfaces and import sorting
* It depends on JDT core, which is required for fomatting.
*/
compile("org.eclipse.jdt:org.eclipse.jdt.core.manipulation:${VER_ECLIPSE_JDT_CORE_MANIPULATION}") {
exclude group: 'org.eclipse.jdt', module: 'org.eclipse.jdt.launching'
exclude group: 'org.eclipse.platform', module: 'org.eclipse.ant.core'
exclude group: 'org.eclipse.platform', module: 'org.eclipse.core.expressions'
}
/*
* JDT UI required for clean-up.
* Only the org.eclipse.jdt.internal.corext.fix package is required.
* All dependencies (like SWT) are excluded.
*/
compile("org.eclipse.jdt:org.eclipse.jdt.ui:${VER_ECLIPSE_JDT_UI}") {
exclude group: 'org.eclipse.platform'
exclude group: 'org.eclipse.jdt'
}
}
5 changes: 3 additions & 2 deletions _ext/eclipse-jdt/gradle.properties
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Mayor/Minor versions correspond to the minimum Eclipse version supported/tested.
# Patch version is incremented for backward compatible patches of this library.
ext_version=4.11.0
ext_version=4.12.0
ext_artifactId=spotless-eclipse-jdt
ext_description=Eclipse's JDT formatter bundled for Spotless

Expand All @@ -12,4 +12,5 @@ ext_VER_JAVA=1.8

# Compile
VER_ECLIPSE_JDT_CORE_MANIPULATION=[1.11.0,2.0.0[
VER_SPOTLESS_ECLISPE_BASE=[3.0.0,4.0.0[
VER_ECLIPSE_JDT_UI=[3.18.0,4.0.0[
VER_SPOTLESS_ECLISPE_BASE=[3.2.0,4.0.0[
Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
/*
* Copyright 2016 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.diffplug.spotless.extra.eclipse.java;

import java.io.IOException;
import java.io.InputStream;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.net.URL;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Properties;
import java.util.Set;
import java.util.stream.Collectors;

import javax.xml.parsers.ParserConfigurationException;
import javax.xml.parsers.SAXParser;
import javax.xml.parsers.SAXParserFactory;
import javax.xml.xpath.XPathExpressionException;

import org.assertj.core.util.Sets;

This comment has been minimized.

Copy link
@jbduncan

jbduncan Aug 28, 2019

Member

@fvgh Did you mean to depend on AssertJ here by any chance? :)

This comment has been minimized.

Copy link
@fvgh

fvgh Aug 28, 2019

Author Member

Sorry, missed your comment here... Yes, there are still some unused imports... It's only a very first draft... Does not build, not all tests working yet.

import org.eclipse.jdt.internal.corext.fix.CleanUpConstants;
import org.eclipse.jdt.internal.corext.fix.CleanUpConstantsOptions;
import org.eclipse.jdt.ui.cleanup.CleanUpOptions;
import org.eclipse.jdt.ui.cleanup.ICleanUp;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.xml.sax.Attributes;
import org.xml.sax.SAXException;
import org.xml.sax.helpers.DefaultHandler;

/** Provides configured clean-up implementations. */
final class CleanUpFactory {

private final static Set<String> UNSUPPORTED_CLASSES = Collections.unmodifiableSet(Sets.newLinkedHashSet(
"org.eclipse.jdt.internal.ui.fix.UnimplementedCodeCleanUp" //Would require Eclipse templates
));

@SuppressWarnings("serial")
private final static Map<String, FixedValue> UNSUPPORTED_CONFIG = Collections.unmodifiableMap(new HashMap<String, FixedValue>() {
{
put(CleanUpConstants.REMOVE_UNUSED_CODE_IMPORTS, new FixedValue("false", "Unused import clean-up only works in case all imports can be resolved. As an alternative use: " + CleanUpConstants.ORGANIZE_IMPORTS));
}
});

private final static String CLEAN_UP_CONFIG_FILE_NAME = "plugin.xml";
private final static String CLEAN_UP_CONFIG_DEPENDENCY_NAME = "org.eclipse.jdt.ui";
private static List<Constructor<? extends ICleanUp>> CLEAN_UP_SEQUENCE = null;
private final CleanUpOptions options;

CleanUpFactory(Properties settings) {
options = new CleanUpOptions();
Logger logger = LoggerFactory.getLogger(CleanUpFactory.class);
CleanUpConstantsOptions.setDefaultOptions(CleanUpConstants.DEFAULT_CLEAN_UP_OPTIONS, options);
UNSUPPORTED_CONFIG.entrySet().stream().forEach(entry -> options.setOption(entry.getKey(), entry.getValue().value));
settings.forEach((key, value) -> {
FixedValue fixed = UNSUPPORTED_CONFIG.get(key);
if (null != fixed && fixed.value != value) {
logger.warn(String.format("Using %s for %s instead of %s: %s", fixed.value, key, value, fixed.reason));
} else {
options.setOption(key.toString(), value.toString());
}
});
try {
initializeCleanupActions();
} catch (IOException | ParserConfigurationException | XPathExpressionException e) {
throw new RuntimeException("Faild to read Eclipse Clean-Up configuration.", e);
}
}

private static synchronized void initializeCleanupActions() throws IOException, ParserConfigurationException, XPathExpressionException {
if (null != CLEAN_UP_SEQUENCE) {
return;
}
ClassLoader loader = CleanUpFactory.class.getClassLoader();
Optional<URL> configUrl = Collections.list(loader.getResources(CLEAN_UP_CONFIG_FILE_NAME)).stream().filter(url -> url.getPath().contains(CLEAN_UP_CONFIG_DEPENDENCY_NAME)).findAny();
if (!configUrl.isPresent()) {
throw new RuntimeException("Could not find JAR containing " + CLEAN_UP_CONFIG_DEPENDENCY_NAME + ":" + CLEAN_UP_CONFIG_FILE_NAME);
}
InputStream configXmlStream = configUrl.get().openStream();
try {
SAXParser saxParser = SAXParserFactory.newInstance().newSAXParser();
CleanUpExtensionHandler handler = new CleanUpExtensionHandler();
saxParser.parse(configXmlStream, handler);
CLEAN_UP_SEQUENCE = handler.getCleanUpSequence();
} catch (SAXException e) {
//Add information about the XML location
throw new RuntimeException("Failed to parse " + configUrl.get().toExternalForm(), e);
}
}

public List<ICleanUp> create() {
return CLEAN_UP_SEQUENCE.stream().map(constructor -> {
try {
ICleanUp cleanUp = constructor.newInstance();
cleanUp.setOptions(options);
return cleanUp;
} catch (InstantiationException | IllegalAccessException | IllegalArgumentException | InvocationTargetException e) {
throw new RuntimeException("Failed to created clean-up action for " + constructor.getName(), e);
}
}).collect(Collectors.toList());
}

private static class FixedValue {
public final String value;
public final String reason;

FixedValue(String value, String reason) {
this.value = value;
this.reason = reason;
}
};

private final static class CleanUpExtensionHandler extends DefaultHandler {
private final static String CLEAN_UP_ELEMENT_NAME = "cleanUp";
private final static String ID_ATTRIBUTE_NAME = "id";
private final static String CLASS_ATTRIBUTE_NAME = "class";
private final static String RUN_AFTER_ATTRIBUTE_NAME = "runAfter";
private final Map<String, Constructor<? extends ICleanUp>> constructor;
private final Map<String, String> runAfter;
private final LinkedList<String> sorted;

CleanUpExtensionHandler() {
constructor = new HashMap<>();
runAfter = new LinkedHashMap<>(); //E.g. the elements are already sorted
sorted = new LinkedList<>();
}

@Override
public void startDocument() throws SAXException {
constructor.clear();
runAfter.clear();
sorted.clear();
super.startDocument();
}

@Override
public void startElement(String uri, String localName, String qName, Attributes attributes) throws SAXException {
if (CLEAN_UP_ELEMENT_NAME == qName) {
String id = getMandatoryAttribute(attributes, ID_ATTRIBUTE_NAME);
String className = getMandatoryAttribute(attributes, CLASS_ATTRIBUTE_NAME);
if (!UNSUPPORTED_CLASSES.contains(className)) {
try {
Class<?> clazz = Class.forName(className);
Class<? extends ICleanUp> clazzImplementsICleanUp = clazz.asSubclass(ICleanUp.class);
constructor.put(id, clazzImplementsICleanUp.getConstructor());
} catch (ClassNotFoundException | ClassCastException | NoSuchMethodException | SecurityException e) {
throw new SAXException("Failed to obtain constructor for " + CLEAN_UP_ELEMENT_NAME + " element class " + className, e);
}
}
String runAfterId = attributes.getValue(RUN_AFTER_ATTRIBUTE_NAME);
if (null == runAfterId) {
sorted.push(id);
} else {
runAfter.put(id, runAfterId);
}
}
super.startElement(uri, localName, qName, attributes);
}

private static String getMandatoryAttribute(Attributes attributes, String qName) throws SAXException {
String value = attributes.getValue(qName);
if (null == value) {
throw new SAXException(CLEAN_UP_ELEMENT_NAME + " element without " + qName + " attribute.");
}
return value;
}

@Override
public void endDocument() throws SAXException {
if (runAfter.isEmpty()) {
throw new SAXException(CLEAN_UP_ELEMENT_NAME + " element has not been found in XML.");
}
while (!runAfter.isEmpty()) {
//E.g. the elements are already sorted. Hence only one iteration is expected.
List<String> foundEntries = new ArrayList<>(runAfter.size());
for (Map.Entry<String, String> entry : runAfter.entrySet()) {
int runAfterIndex = sorted.lastIndexOf(entry.getValue());
if (0 <= runAfterIndex) {
foundEntries.add(entry.getKey());
sorted.add(runAfterIndex + 1, entry.getKey());
}
}
foundEntries.forEach(e -> runAfter.remove(e));
if (foundEntries.isEmpty()) {
throw new SAXException(CLEAN_UP_ELEMENT_NAME + " element the following precessor IDs cannot be resolved: " + runAfter.values().stream().collect(Collectors.joining("; ")));
}
}
super.endDocument();
}

public List<Constructor<? extends ICleanUp>> getCleanUpSequence() {
return sorted.stream().map(id -> constructor.get(id)).filter(clazz -> null != clazz).collect(Collectors.toList());
}
}

}

2 comments on commit 2085159

@fvgh
Copy link
Member Author

@fvgh fvgh commented on 2085159 Aug 28, 2019

Choose a reason for hiding this comment

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

@jbduncan: Probably I'll use AssertJ to avoid catching ComparisonFailure. Refactoring is easy. But first I must do a little bit more evaluation, whether the clean-up can really be used without a project scope. Makes no sense to polish when I find in the end that the whole approach was a waste of time (well, despite learning more about Java and Eclipse).

@jbduncan
Copy link
Member

Choose a reason for hiding this comment

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

@fvgh Understood!

The reason why I raised the AssertJ usage above was because it was in a src/main/java file, which suggested to me that we were accidentally introducing AssertJ as a compile dependency rather than a test one. But let's talk about it later if this change sees the light of day. :)

Please sign in to comment.