Skip to content

Enforce var jdk11 applied #1609

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -168,7 +168,7 @@ public Bundle getBundle(String location) {
@Override
public ServiceReference<?>[] getAllServiceReferences(String clazz, String filter) throws InvalidSyntaxException {
//Filters are based on class names
String interfaceClassName = (null == clazz) ? filter : clazz;
var interfaceClassName = (null == clazz) ? filter : clazz;
return services.getReferences(interfaceClassName);
}

Original file line number Diff line number Diff line change
@@ -63,7 +63,7 @@ class ResourceAccessor {
/** Resources are located in the JAR of the given class
* @throws BundleException */
ResourceAccessor(Class<?> clazz) throws BundleException {
String bundleObjPath = clazz.getName();
var bundleObjPath = clazz.getName();
fatJarResourcePath = bundleObjPath.substring(0, bundleObjPath.lastIndexOf('.'));
try {
bundleFile = getBundlFile(clazz);
@@ -74,7 +74,7 @@ class ResourceAccessor {

private static BundleFile getBundlFile(Class<?> clazz) throws BundleException {
URI objUri = getBundleUri(clazz);
File jarOrDirectory = new File(objUri.getPath());
var jarOrDirectory = new File(objUri.getPath());
Copy link
Contributor

@thc202 thc202 Mar 7, 2023

Choose a reason for hiding this comment

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

Why wasn't the URI above replaced?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At first sight, it would be a limitation around type resolution, due to file being processed one a per-file basis. Let me have a deeper look.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

UnsolvedSymbolException{context='null', name='BundleException', cause='null'}. One may argue this information is not relevant in this purpose. However, here it is: JavaParser, the source -> AST used to manipulate Java-Code failed resolving the type of getBundleUri(clazz) as given method refers to a type unknown in the context of given source file.

Similar technical challenge around #1379 (comment).

For now, Cleanthat focuses on linting on a per-file basis (for similar reason Spotless processes (for now) each file individually).

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the explanation!

Copy link
Member

Choose a reason for hiding this comment

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

I would think that any assignment could be a var and we don't need to know anything about its type. The only reason not to var is if it's a declaration without an assignment. Am I wrong?

Copy link
Contributor Author

@blacelle blacelle Mar 9, 2023

Choose a reason for hiding this comment

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

You can find the unitTests for this mutator in LocalVariableTypeInferenceCases. There is multiple cases with @UnmodifiedMethod, expressing switching to a var would be invalid.

Even List<?> i = new ArrayList<String>(); should not be switched to var as you may later assign a CopyOnWriteArrayList while var would have turned semantically from List to ArrayList (and you can't assign a CopyOnWriteArrayList into an ArrayList).

We may then suppose final List<?> i = new ArrayList<String>(); could be turned to var, however I may be missing another edge-case, which would lead in Cleanthat producing invalid code.

if (!(jarOrDirectory.exists() && jarOrDirectory.canRead())) {
throw new BundleException(String.format("Path '%s' for '%s' is not accessible exist on local file system.", objUri, clazz.getName()), BundleException.READ_ERROR);
}
@@ -87,7 +87,7 @@ private static BundleFile getBundlFile(Class<?> clazz) throws BundleException {

private static URI getBundleUri(Class<?> clazz) throws BundleException {
try {
URL objUrl = clazz.getProtectionDomain().getCodeSource().getLocation();
var objUrl = clazz.getProtectionDomain().getCodeSource().getLocation();
return objUrl.toURI();
} catch (NullPointerException e) {
//No bunlde should be used for RT classes lookup. See also org.eclipse.core.runtime.PerformanceStats.
@@ -101,10 +101,10 @@ private static URI getBundleUri(Class<?> clazz) throws BundleException {

/** Get the manifest name from the resources. */
String getManifestName() throws BundleException {
URL manifestUrl = getEntry(JarFile.MANIFEST_NAME);
var manifestUrl = getEntry(JarFile.MANIFEST_NAME);
if (null != manifestUrl) {
try {
Manifest manifest = new Manifest(manifestUrl.openStream());
var manifest = new Manifest(manifestUrl.openStream());
String headerValue = manifest.getMainAttributes().getValue(Constants.BUNDLE_SYMBOLICNAME);
if (null == headerValue) {
throw new BundleException(String.format("Symbolic values not found in '%s'.", manifestUrl), BundleException.MANIFEST_ERROR);
Original file line number Diff line number Diff line change
@@ -76,7 +76,7 @@ public void set(String key, String value) {

@Override
public <S> void add(Class<S> interfaceClass, S service) throws ServiceException {
String className = interfaceClass.getName();
var className = interfaceClass.getName();
if (null != className2Service.put(interfaceClass.getName(), new FrameworkServiceReference<S>(className, service))) {
throw new ServiceException(
String.format("Service '%s' is already registered.", interfaceClass.getName()), ServiceException.FACTORY_ERROR);
Original file line number Diff line number Diff line change
@@ -39,7 +39,7 @@ public class PluginRegistrar {
private static final String PLUGIN_PROPERTIES = "plugin.properties";

public static BundleException register(Bundle bundle) {
PluginRegistrar registrar = new PluginRegistrar(bundle);
var registrar = new PluginRegistrar(bundle);
try {
registrar.register();
} catch (BundleException e) {
Original file line number Diff line number Diff line change
@@ -459,7 +459,7 @@ public long getTime() {

@Override
public String toString() {
StringWriter result = new StringWriter();
var result = new StringWriter();
result.write(message);
if (execption.isPresent()) {
result.write('\n');
Original file line number Diff line number Diff line change
@@ -44,7 +44,7 @@ private TemporaryLocation(Location parent, URL defaultValue) {

private static URL createTemporaryDirectory() {
try {
Path location = Files.createTempDirectory(TEMP_PREFIX);
var location = Files.createTempDirectory(TEMP_PREFIX);
return location.toUri().toURL();
} catch (IOException e) {
throw new IOError(e);
@@ -120,7 +120,7 @@ public Location createLocation(Location parent, URL defaultValue, boolean readon
@Override
public URL getDataArea(String path) throws IOException {
try {
Path locationPath = Paths.get(location.toURI());
var locationPath = Paths.get(location.toURI());
return locationPath.resolve(path).toUri().toURL();
} catch (URISyntaxException e) {
throw new IOException("Location not correctly formatted.", e);
@@ -130,7 +130,7 @@ public URL getDataArea(String path) throws IOException {
@Override
public void close() throws Exception {
try {
Path path = Path.of(location.toURI());
var path = Path.of(location.toURI());
Files.walk(path)
.sorted(Comparator.reverseOrder())
.map(Path::toFile)
Original file line number Diff line number Diff line change
@@ -251,7 +251,7 @@ public void println(String x) {
@Override
public void println(Object x) {
if (x instanceof Exception) {
Exception e = (Exception) x;
var e = (Exception) x;
if (TEST_EXCEPTION_MESSAGE == e.getMessage()) {
messages.add(TEST_EXCEPTION_MESSAGE);
}
Original file line number Diff line number Diff line change
@@ -49,7 +49,7 @@ void testAddGet() throws BundleException {

@Test
void testSameSymbolicName() throws BundleException {
final String symbolicName = "sym.a";
final var symbolicName = "sym.a";
final long id1 = 12345;
final long id2 = 23456;
Bundle testBundle1 = new TestBundle(id1, symbolicName);
@@ -63,8 +63,8 @@ void testSameSymbolicName() throws BundleException {

@Test
void testSameID() throws BundleException {
final String symbolicName1 = "sym.a";
final String symbolicName2 = "sym.b";
final var symbolicName1 = "sym.a";
final var symbolicName2 = "sym.b";
final long id = 12345;
Bundle testBundle1 = new TestBundle(id, symbolicName1);
Bundle testBundle2 = new TestBundle(id, symbolicName2);
Original file line number Diff line number Diff line change
@@ -39,8 +39,8 @@ void initialize() {

@Test
void testAddGet() {
Service1 service1 = new Service1();
Service2 service2 = new Service2();
var service1 = new Service1();
var service2 = new Service2();
instance.add(Interf1.class, service1);
instance.add(Interf2a.class, service2);
instance.add(Interf2b.class, service2);
@@ -51,8 +51,8 @@ void testAddGet() {

@Test
void testMultipleServicesPerInterface() {
Service1 serviceX = new Service1();
Service1 serviceY = new Service1();
var serviceX = new Service1();
var serviceY = new Service1();
instance.add(Interf1.class, serviceX);
ServiceException e = assertThrows(ServiceException.class, () -> instance.add(Interf1.class, serviceY));
assertThat(e.getMessage()).as("ServiceException does not contain interface class name.").contains(Interf1.class.getName());
Original file line number Diff line number Diff line change
@@ -141,7 +141,7 @@ public void activatePlugins(SpotlessEclipsePluginConfig config) {
* The HTML formatter only uses the DOCTYPE/SCHEMA for content model selection.
* Hence no external URIs are required.
*/
boolean allowExternalURI = false;
var allowExternalURI = false;
EclipseXmlFormatterStepImpl.FrameworkConfig.activateXmlPlugins(config, allowExternalURI);
config.add(new HTMLCorePlugin());
}
Original file line number Diff line number Diff line change
@@ -63,7 +63,7 @@ public StructuredDocumentProcessor(IStructuredDocument document, String type,

/** Applies processor on document, using a given formatter */
public void apply(T formatter) {
for (int currentRegionId = 0; currentRegionId < numberOfRegions; currentRegionId++) {
for (var currentRegionId = 0; currentRegionId < numberOfRegions; currentRegionId++) {
applyOnRegion(currentRegionId, formatter);
}
}
@@ -132,7 +132,7 @@ public int getLastLine() {
}

private static int computeIndent(IStructuredDocument document, ITypedRegion region, String htmlIndent) {
int indent = 0;
var indent = 0;
try {
int lineNumber = document.getLineOfOffset(region.getOffset());
document.getNumberOfLines();
@@ -180,7 +180,7 @@ protected void fixTagIndent(MultiTextEdit modifications, int offset, String inde
int lineStart = document.getLineOffset(lineNumber);
int lineEnd = document.getLineOffset(lineNumber + 1);
String lineContent = document.get(lineStart, lineEnd - lineStart);
StringBuilder currentIndent = new StringBuilder();
var currentIndent = new StringBuilder();
lineContent.chars().filter(c -> {
if (c == ' ' || c == '\t') {
currentIndent.append(c);
Original file line number Diff line number Diff line change
@@ -96,9 +96,9 @@ public static void configureCatalog(final Properties properties, final ICatalog
throw new IllegalArgumentException("Internal error: Catalog implementation '" + defaultCatalogInterface.getClass().getCanonicalName() + "' unsupported.");
}
Catalog defaultCatalog = (Catalog) defaultCatalogInterface;
String catalogProperty = properties.getProperty(USER_CATALOG, "");
var catalogProperty = properties.getProperty(USER_CATALOG, "");
if (!catalogProperty.isEmpty()) {
final File catalogFile = new File(catalogProperty);
final var catalogFile = new File(catalogProperty);
try {
InputStream inputStream = new FileInputStream(catalogFile);
String orgBase = defaultCatalog.getBase();
@@ -118,7 +118,7 @@ public static void configureCatalog(final Properties properties, final ICatalog
public static void assertNoChanges(Plugin plugin, Properties properties) {
Objects.requireNonNull(properties, "Property values are missing.");
final String preferenceId = plugin.getBundle().getSymbolicName();
Properties originalValues = CONFIG.get(preferenceId);
var originalValues = CONFIG.get(preferenceId);
if (null == originalValues) {
throw new IllegalArgumentException("No configuration found for " + preferenceId);
}
Original file line number Diff line number Diff line change
@@ -41,7 +41,7 @@ void initialize() throws Exception {
* All formatter configuration is stored in
* org.eclipse.core.runtime/.settings/org.eclipse.wst.css.core.prefs.
*/
Properties properties = new Properties();
var properties = new Properties();
properties.put(INDENTATION_SIZE, "3");
properties.put(INDENTATION_CHAR, SPACE); //Default is TAB
properties.put(CLEANUP_CASE_SELECTOR, Integer.toString(UPPER)); //Done by cleanup
Original file line number Diff line number Diff line change
@@ -40,7 +40,7 @@ void initialize() throws Exception {
* org.eclipse.core.runtime/.settings/org.eclipse.wst.xml.core.prefs.
* So a simple test of one configuration item change is considered sufficient.
*/
Properties properties = new Properties();
var properties = new Properties();
properties.put(CLEANUP_TAG_NAME_CASE, Integer.toString(HTMLCorePreferenceNames.UPPER)); //HTML config
properties.put(FORMATTER_INSERT_SPACE_BEFORE_SEMICOLON, JavaScriptCore.INSERT); //JS config
properties.put(QUOTE_ATTR_VALUES, "TRUE"); //CSS config
@@ -89,7 +89,7 @@ void formatCSS() throws Exception {

@Test
void checkCleanupForNonUtf8() throws Exception {
String osEncoding = System.getProperty("file.encoding");
var osEncoding = System.getProperty("file.encoding");
System.setProperty("file.encoding", "ISO-8859-1"); //Simulate a non UTF-8 OS
String[] input = testData.input("utf-8.html");
String output = formatter.format(input[0]);
Original file line number Diff line number Diff line change
@@ -50,7 +50,7 @@ void initialize() throws Exception {
* All formatter configuration is stored in
* org.eclipse.core.runtime/.settings/org.eclipse.jst.jsdt.core.prefs.
*/
Properties properties = new Properties();
var properties = new Properties();
properties.setProperty(DefaultCodeFormatterConstants.FORMATTER_TAB_CHAR, JavaScriptCore.TAB);
formatter = new EclipseJsFormatterStepImpl(properties);
}
Original file line number Diff line number Diff line change
@@ -41,7 +41,7 @@ void initialize() throws Exception {
* org.eclipse.core.runtime/.settings/org.eclipse.wst.json.core.prefs.
* So a simple test of one configuration item change is considered sufficient.
*/
Properties properties = new Properties();
var properties = new Properties();
properties.put(INDENTATION_SIZE, "3"); //Default is 1
properties.put(INDENTATION_CHAR, SPACE); //Default is TAB
properties.put(CASE_PROPERTY_NAME, Integer.toString(UPPER)); //Dead code, ignored
Original file line number Diff line number Diff line change
@@ -41,7 +41,7 @@ void initializeStatic() throws Exception {
* org.eclipse.core.runtime/.settings/org.eclipse.wst.xml.core.prefs.
* So a simple test of one configuration item change is considered sufficient.
*/
Properties properties = new Properties();
var properties = new Properties();
properties.put(PluginPreferences.RESOLVE_EXTERNAL_URI, "TRUE");
properties.put(INDENTATION_SIZE, "2");
properties.put(INDENTATION_CHAR, SPACE); //Default is TAB
Original file line number Diff line number Diff line change
@@ -41,7 +41,7 @@ void initializeStatic() throws Exception {
* org.eclipse.core.runtime/.settings/org.eclipse.wst.xml.core.prefs.
* So a simple test of one configuration item change is considered sufficient.
*/
Properties properties = new Properties();
var properties = new Properties();
properties.put(INDENTATION_SIZE, "2");
properties.put(INDENTATION_CHAR, SPACE); //Default is TAB
properties.put(PluginPreferences.USER_CATALOG, testData.getRestrictionsPath("catalog.xml").toString());
Original file line number Diff line number Diff line change
@@ -42,7 +42,7 @@ void initialize() throws Exception {
* org.eclipse.core.runtime/.settings/org.eclipse.wst.xml.core.prefs.
* So a simple test of one configuration item change is considered sufficient.
*/
Properties properties = new Properties();
var properties = new Properties();
properties.put(INDENTATION_SIZE, "2");
properties.put(INDENTATION_CHAR, SPACE); //Default is TAB
formatter = new EclipseXmlFormatterStepImpl(properties);
Original file line number Diff line number Diff line change
@@ -22,8 +22,8 @@

public class TestData {
public static TestData getTestDataOnFileSystem(String kind) {
final String userDir = System.getProperty("user.dir", ".");
Path dataPath = Paths.get(userDir, "src", "test", "resources", kind);
final var userDir = System.getProperty("user.dir", ".");
var dataPath = Paths.get(userDir, "src", "test", "resources", kind);
if (Files.isDirectory(dataPath)) {
return new TestData(dataPath);
}
@@ -46,12 +46,12 @@ private TestData(Path dataPath) {
}

public String[] input(final String fileName) throws Exception {
Path xmlPath = inputPath.resolve(fileName);
var xmlPath = inputPath.resolve(fileName);
return new String[]{read(xmlPath), xmlPath.toString()};
}

public String expected(final String fileName) {
Path xmlPath = expectedPath.resolve(fileName);
var xmlPath = expectedPath.resolve(fileName);
return read(xmlPath);
}

@@ -60,15 +60,15 @@ private String read(final Path xmlPath) {
throw new IllegalArgumentException(String.format("'%1$s' is not a regular file.", xmlPath));
}
try {
String checkedOutFileContent = new String(java.nio.file.Files.readAllBytes(xmlPath), "UTF8");
var checkedOutFileContent = new String(java.nio.file.Files.readAllBytes(xmlPath), "UTF8");
return checkedOutFileContent.replace("\r", ""); //Align GIT end-of-line normalization
} catch (IOException e) {
throw new IllegalArgumentException(String.format("Failed to read '%1$s'.", xmlPath), e);
}
}

public Path getRestrictionsPath(String fileName) {
Path filePath = restrictionsPath.resolve(fileName);
var filePath = restrictionsPath.resolve(fileName);
if (!Files.exists(filePath)) {
throw new IllegalArgumentException(String.format("'%1$s' is not a restrictions file.", fileName));
}
4 changes: 4 additions & 0 deletions gradle/spotless.gradle
Original file line number Diff line number Diff line change
@@ -17,6 +17,10 @@ spotless {
java {
ratchetFrom 'origin/main'
bumpThisNumberIfACustomStepChanges(1)
// `setMutators` will discard default mutators
// https://sonarsource.atlassian.net/browse/RSPEC-6212
cleanthat().version("2.9").sourceCompatibility(project.sourceCompatibility.toString()).clearMutators()
Copy link
Member

Choose a reason for hiding this comment

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

Maybe the sourceCompatibility could be set here?

if (isLicenseHeaderStep(step)) {
return step.filterByFile(LicenseHeaderStep.unsupportedJvmFilesFilter());
} else {

Something like if (isCleanThatStep(step)) && sourceCompatibility == null) { ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not suceed doing so in the initial iteration. having a new look with your suggestion, I'm still stuck as I can not find sourceCompatiblity in Project: https://github.com/gradle/gradle/blob/v7.6.1/subprojects/core-api/src/main/java/org/gradle/api/Project.java, there is no sourceCompatiblity.

Yet, project.sourceCompatibility is OK in gradle files. What am I missing here?

Copy link
Member

Choose a reason for hiding this comment

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

.addMutator('RSPEC-6212')
licenseHeaderFile rootProject.file('gradle/spotless.license')
importOrderFile rootProject.file('gradle/spotless.importorder')
eclipse().configFile rootProject.file('gradle/spotless.eclipseformat.xml')
Original file line number Diff line number Diff line change
@@ -89,7 +89,7 @@ public GrEclipseFormatterStepImpl(final Properties properties) throws Exception
/** Formatting Groovy string */
public String format(String raw) throws Exception {
IDocument doc = new Document(raw);
GroovyErrorListener errorListener = new GroovyErrorListener();
var errorListener = new GroovyErrorListener();
TextSelection selectAll = new TextSelection(doc, 0, doc.getLength());
GroovyFormatter codeFormatter = new DefaultGroovyFormatter(selectAll, doc, preferencesStore, false);
TextEdit edit = codeFormatter.format();
@@ -135,7 +135,7 @@ public boolean errorsDetected() {

@Override
public String toString() {
StringBuilder string = new StringBuilder();
var string = new StringBuilder();
if (1 < errors.size()) {
string.append("Multiple problems detected during step execution:");
} else if (0 == errors.size()) {
@@ -168,9 +168,9 @@ public void log(TraceCategory arg0, String arg1) {

private static PreferenceStore createPreferences(final Properties properties) throws IOException {
final PreferenceStore preferences = new PreferenceStore();
ByteArrayOutputStream output = new ByteArrayOutputStream();
var output = new ByteArrayOutputStream();
properties.store(output, null);
ByteArrayInputStream input = new ByteArrayInputStream(output.toByteArray());
var input = new ByteArrayInputStream(output.toByteArray());
preferences.load(input);
return preferences;
}
Loading