Config import: change bubble iteration

src/core/lombok/core/LombokConfiguration.java

@@ -25,6 +25,7 @@ import java.net.URI;
 import java.util.Collections;
 
 import lombok.core.configuration.BubblingConfigurationResolver;
+import lombok.core.configuration.ConfigurationFileToSource;
 import lombok.core.configuration.ConfigurationKey;
 import lombok.core.configuration.ConfigurationParser;
 import lombok.core.configuration.ConfigurationProblemReporter;
@@ -65,7 +66,7 @@ public class LombokConfiguration {
 	}
 	
 	static <T> T read(ConfigurationKey<T> key, AST<?, ?, ?> ast) {
-		return configurationResolverFactory.createResolver(ast.getAbsoluteFileLocation()).resolve(key);
+		return read(key, ast.getAbsoluteFileLocation());
 	}
 	
 	public static <T> T read(ConfigurationKey<T> key, URI sourceLocation) {
@@ -73,9 +74,10 @@ public class LombokConfiguration {
 	}
 	
 	private static ConfigurationResolverFactory createFileSystemBubblingResolverFactory() {
+		final ConfigurationFileToSource fileToSource = cache.fileToSource(new ConfigurationParser(ConfigurationProblemReporter.CONSOLE));
 		return new ConfigurationResolverFactory() {
 			@Override public ConfigurationResolver createResolver(URI sourceLocation) {
-				return new BubblingConfigurationResolver(cache.sourcesForJavaFile(sourceLocation, new ConfigurationParser(ConfigurationProblemReporter.CONSOLE)));
+				return new BubblingConfigurationResolver(cache.forUri(sourceLocation), fileToSource);
 			}
 		};
 	}

src/core/lombok/core/configuration/BubblingConfigurationResolver.java

@@ -21,19 +21,26 @@
  */
 package lombok.core.configuration;
 
+import java.util.ArrayDeque;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
+import java.util.Deque;
+import java.util.HashSet;
 import java.util.List;
 
+import lombok.ConfigurationKeys;
 import lombok.core.configuration.ConfigurationSource.ListModification;
 import lombok.core.configuration.ConfigurationSource.Result;
 
 public class BubblingConfigurationResolver implements ConfigurationResolver {
 	
-	private final Iterable<ConfigurationSource> sources;
+	private final ConfigurationFile start;
+	private final ConfigurationFileToSource fileMapper;
 	
-	public BubblingConfigurationResolver(Iterable<ConfigurationSource> sources) {
-		this.sources = sources;
+	public BubblingConfigurationResolver(ConfigurationFile start, ConfigurationFileToSource fileMapper) {
+		this.start = start;
+		this.fileMapper = fileMapper;
 	}
 	
 	@SuppressWarnings("unchecked")
@@ -41,20 +48,43 @@ public class BubblingConfigurationResolver implements ConfigurationResolver {
 	public <T> T resolve(ConfigurationKey<T> key) {
 		boolean isList = key.getType().isList();
 		List<List<ListModification>> listModificationsList = null;
+
+		boolean stopBubbling = false;
+		ConfigurationFile currentLevel = start;
+		Collection<ConfigurationFile> visited = new HashSet<ConfigurationFile>();
 		outer:
-		for (ConfigurationSource source : sources) {
-			source.imports();
-			Result result = source.resolve(key);
-			if (result == null) continue;
-			if (isList) {
-				if (listModificationsList == null) listModificationsList = new ArrayList<List<ListModification>>();
-				listModificationsList.add((List<ListModification>)result.getValue());
-			}
-			if (result.isAuthoritative()) {
-				if (isList) break outer;
-				return (T) result.getValue();
+		while (!stopBubbling && currentLevel != null) {
+			Deque<ConfigurationFile> round = new ArrayDeque<ConfigurationFile>();
+			round.push(currentLevel);
+			
+			while (!round.isEmpty()) {
+				ConfigurationFile currentFile = round.pop();
+				if (currentFile == null || !visited.add(currentFile)) continue;
+				
+				ConfigurationSource source = fileMapper.parsed(currentFile);
+				if (source == null) continue;
+				
+				for (ConfigurationFile importFile : source.imports()) round.push(importFile);
+				
+				Result stop = source.resolve(ConfigurationKeys.STOP_BUBBLING);
+				stopBubbling = stopBubbling || (stop != null && Boolean.TRUE.equals(stop.getValue()));
+				
+				Result result = source.resolve(key);
+				if (result == null) {
+					continue;
+				}
+				if (isList) {
+					if (listModificationsList == null) listModificationsList = new ArrayList<List<ListModification>>();
+					listModificationsList.add((List<ListModification>)result.getValue());
+				}
+				if (result.isAuthoritative()) {
+					if (isList) break outer;
+					return (T) result.getValue();
+				}
 			}
+			currentLevel = currentLevel.parent();
 		}
+		
 		if (!isList) return null;
 		if (listModificationsList == null) return (T) Collections.emptyList();
 		

src/core/lombok/core/configuration/ConfigurationApp.java

@@ -204,7 +204,7 @@ public class ConfigurationApp extends LombokApp {
 				out.println();
 			}
 			URI directory = entry.getKey();
-			ConfigurationResolver resolver = new BubblingConfigurationResolver(cache.sourcesForDirectory(directory, parser));
+			ConfigurationResolver resolver = new BubblingConfigurationResolver(cache.forUri(directory), cache.fileToSource(parser));
 			Map<ConfigurationKey<?>, ? extends Collection<String>> traces = trace(keys, directory);
 			boolean printed = false;
 			for (ConfigurationKey<?> key : keys) {
@@ -255,10 +255,9 @@ public class ConfigurationApp extends LombokApp {
 		boolean stopBubbling = false;
 		String previousDescription = null;
 		for (File currentDirectory = new File(directory); currentDirectory != null && !stopBubbling; currentDirectory = currentDirectory.getParentFile()) {
-			File configFile = new File(currentDirectory, "lombok.config");
-			if (!configFile.exists() || !configFile.isFile()) continue;
+			ConfigurationFile context = ConfigurationFile.forDirectory(currentDirectory);
+			if (!context.exists()) continue;
 			
-			ConfigurationFile context = ConfigurationFile.fromFile(configFile);
 			Map<ConfigurationKey<?>, List<String>> traces = trace(context, keys);
 			
 			stopBubbling = stopBubbling(traces.get(ConfigurationKeys.STOP_BUBBLING));
@@ -293,7 +292,7 @@ public class ConfigurationApp extends LombokApp {
 		
 		Collector collector = new Collector() {
 			@Override public void addImport(ConfigurationFile importFile, ConfigurationFile context, int lineNumber) {
-				// TODO Trace imports
+				// nothing to display here
 			}
 			@Override public void clear(ConfigurationKey<?> key, ConfigurationFile context, int lineNumber) {
 				trace(key, "clear " + key.getKeyName(), lineNumber);

src/core/lombok/core/configuration/ConfigurationFile.java

@@ -28,6 +28,8 @@ import java.io.IOException;
 import java.io.InputStream;
 
 public abstract class ConfigurationFile {
+	private static final String LOMBOK_CONFIG_FILENAME = "lombok.config";
+	
 	private static final ThreadLocal<byte[]> buffers = new ThreadLocal<byte[]>() {
 		protected byte[] initialValue() {
 			return new byte[65536];
@@ -36,10 +38,15 @@ public abstract class ConfigurationFile {
 		
 	private final String identifier;
 	
-	public static ConfigurationFile fromFile(File file) {
+	public static ConfigurationFile forFile(File file) {
 		return new RegularConfigurationFile(file);
 	}
 	
+	
+	public static ConfigurationFile forDirectory(File directory) {
+		return ConfigurationFile.forFile(new File(directory, LOMBOK_CONFIG_FILENAME));
+	}
+	
 	public static ConfigurationFile fromCharSequence(String identifier, CharSequence contents, long lastModified) {
 		return new CharSequenceConfigurationFile(identifier, contents, lastModified);
 	}
@@ -52,6 +59,7 @@ public abstract class ConfigurationFile {
 	abstract boolean exists();
 	abstract CharSequence contents() throws IOException;
 	public abstract ConfigurationFile resolve(String path);
+	abstract ConfigurationFile parent();
 	
 	final String description() {
 		return identifier;
@@ -104,7 +112,7 @@ public abstract class ConfigurationFile {
 		
 		public ConfigurationFile resolve(String path) {
 			File file = resolveFile(path);
-			return file == null ? null : fromFile(file);
+			return file == null ? null : forFile(file);
 		}
 		
 		private File resolveFile(String path) {
@@ -133,6 +141,11 @@ public abstract class ConfigurationFile {
 		CharSequence contents() throws IOException {
 			return read(new FileInputStream(file));
 		}
+
+		@Override ConfigurationFile parent() {
+			File parent = file.getParentFile().getParentFile();
+			return parent == null ? null : forDirectory(parent);
+		}
 	}
 	
 	private static class CharSequenceConfigurationFile extends ConfigurationFile {
@@ -160,5 +173,9 @@ public abstract class ConfigurationFile {
 		@Override public ConfigurationFile resolve(String path) {
 			return null;
 		}
+
+		@Override ConfigurationFile parent() {
+			return null;
+		}
 	}
 }

src/core/lombok/core/configuration/ConfigurationFileToSource.java

@@ -0,0 +1,26 @@
+/*
+ * Copyright (C) 2020 The Project Lombok Authors.
+ * 
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ * 
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ * 
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package lombok.core.configuration;
+
+public interface ConfigurationFileToSource {
+	ConfigurationSource parsed(ConfigurationFile fileLocation);
+}

src/core/lombok/core/configuration/FileSystemSourceCache.java

@@ -23,26 +23,20 @@ package lombok.core.configuration;
 
 import java.io.File;
 import java.net.URI;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.NoSuchElementException;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.TimeUnit;
 
-import lombok.ConfigurationKeys;
-import lombok.core.configuration.ConfigurationSource.Result;
 import lombok.core.debug.ProblemReporter;
 
 public class FileSystemSourceCache {
-	private static final String LOMBOK_CONFIG_FILENAME = "lombok.config";
 	private static final long FULL_CACHE_CLEAR_INTERVAL = TimeUnit.MINUTES.toMillis(30);
 	private static final long RECHECK_FILESYSTEM = TimeUnit.SECONDS.toMillis(2);
 	private static final long NEVER_CHECKED = -1;
 	static final long MISSING = -88; // Magic value; any lombok.config with this exact epochmillis last modified will never be read, so, let's ensure nobody accidentally has one with that exact last modified stamp.
 	
 	private final ConcurrentMap<ConfigurationFile, Content> fileCache = new ConcurrentHashMap<ConfigurationFile, Content>(); // caches files to the content object that tracks content.
-	private final ConcurrentMap<URI, File> uriCache = new ConcurrentHashMap<URI, File>(); // caches URIs of java source files to the dir that contains it.
+	private final ConcurrentMap<URI, ConfigurationFile> uriCache = new ConcurrentHashMap<URI, ConfigurationFile>(); // caches URIs of java source files to the dir that contains it.
 	private volatile long lastCacheClear = System.currentTimeMillis();
 	
 	private void cacheClear() {
@@ -57,19 +51,28 @@ public class FileSystemSourceCache {
 		}
 	}
 	
-	public Iterable<ConfigurationSource> sourcesForJavaFile(URI javaFile, ConfigurationParser parser) {
-		if (javaFile == null) return Collections.emptyList();
+	public ConfigurationFileToSource fileToSource(final ConfigurationParser parser) {
+		return new ConfigurationFileToSource() {
+			@Override public ConfigurationSource parsed(ConfigurationFile fileLocation) {
+				return parseIfNeccesary(fileLocation, parser);
+			}
+		};
+	}
+	
+	public ConfigurationFile forUri(URI javaFile) {
+		if (javaFile == null) return null;
 		cacheClear();
-		File dir = uriCache.get(javaFile);
-		if (dir == null) {
+		ConfigurationFile result = uriCache.get(javaFile);
+		if (result == null) {
 			URI uri = javaFile.normalize();
 			if (!uri.isAbsolute()) uri = URI.create("file:" + uri.toString());
 			
 			try {
 				File file = new File(uri);
 				if (!file.exists()) throw new IllegalArgumentException("File does not exist: " + uri);
-				dir = file.isDirectory() ? file : file.getParentFile();
-				if (dir != null) uriCache.put(javaFile, dir);
+				File directory = file.isDirectory() ? file : file.getParentFile();
+				if (directory != null) result = ConfigurationFile.forDirectory(directory);
+				uriCache.put(javaFile, result);
 			} catch (IllegalArgumentException e) {
 				// This means that the file as passed is not actually a file at all, and some exotic path system is involved.
 				// examples: sourcecontrol://jazz stuff, or an actual relative path (uri.isAbsolute() is completely different, that checks presence of schema!),
@@ -83,84 +86,20 @@ public class FileSystemSourceCache {
 				ProblemReporter.error("Can't find absolute path of file being compiled: " + javaFile, e);
 			}
 		}
-		
-		if (dir != null) {
-			try {
-				return sourcesForDirectory(dir, parser);
-			} catch (Exception e) {
-				// Especially for eclipse's sake, exceptions here make eclipse borderline unusable, so let's play nice.
-				ProblemReporter.error("Can't resolve config stack for dir: " + dir.getAbsolutePath(), e);
-			}
-		}
-		
-		return Collections.emptyList();
+		return result;
 	}
-	
-	public Iterable<ConfigurationSource> sourcesForDirectory(URI directory, ConfigurationParser parser) {
-		return sourcesForJavaFile(directory, parser);
-	}
-	
-	private Iterable<ConfigurationSource> sourcesForDirectory(final File directory, final ConfigurationParser parser) {
-		return new Iterable<ConfigurationSource>() {
-			@Override 
-			public Iterator<ConfigurationSource> iterator() {
-				return new Iterator<ConfigurationSource>() {
-					File currentDirectory = directory;
-					ConfigurationSource next;
-					boolean stopBubbling = false;
-					
-					@Override
-					public boolean hasNext() {
-						if (next != null) return true;
-						if (stopBubbling) return false;
-						next = findNext();
-						return next != null;
-					}
-					
-					@Override
-					public ConfigurationSource next() {
-						if (!hasNext()) throw new NoSuchElementException();
-						ConfigurationSource result = next;
-						next = null;
-						return result;
-					}
-					
-					private ConfigurationSource findNext() {
-						while (currentDirectory != null && next == null) {
-							next = getSourceForDirectory(currentDirectory, parser);
-							currentDirectory = currentDirectory.getParentFile();
-						}
-						if (next != null) {
-							Result stop = next.resolve(ConfigurationKeys.STOP_BUBBLING);
-							stopBubbling = (stop != null && Boolean.TRUE.equals(stop.getValue()));
-						}
-						return next;
-					}
-					
-					@Override
-					public void remove() {
-						throw new UnsupportedOperationException();
-					}
-				};
-			}
-		};
-	}
-	
-	ConfigurationSource getSourceForDirectory(File directory,ConfigurationParser parser) {
-		return getSourceForConfigFile(ConfigurationFile.fromFile(new File(directory, LOMBOK_CONFIG_FILENAME)), parser);
-	}
-	
-	private ConfigurationSource getSourceForConfigFile(ConfigurationFile context, ConfigurationParser parser) {
+
+	private ConfigurationSource parseIfNeccesary(ConfigurationFile file, ConfigurationParser parser) {
 		long now = System.currentTimeMillis();
-		Content content = ensureContent(context);
+		Content content = ensureContent(file);
 		synchronized (content) {
 			if (content.lastChecked != NEVER_CHECKED && now - content.lastChecked < RECHECK_FILESYSTEM) {
 				return content.source;
 			}
 			content.lastChecked = now;
 			long previouslyModified = content.lastModified;
-			content.lastModified = context.getLastModifiedOrMissing();
-			if (content.lastModified != previouslyModified) content.source = content.lastModified == MISSING ? null : parse(context, parser);
+			content.lastModified = file.getLastModifiedOrMissing();
+			if (content.lastModified != previouslyModified) content.source = content.lastModified == MISSING ? null : SingleConfigurationSource.parse(file, parser);
 			return content.source;
 		}
 	}
@@ -174,10 +113,6 @@ public class FileSystemSourceCache {
 		return fileCache.get(context);
 	}
 	
-	private ConfigurationSource parse(ConfigurationFile context, ConfigurationParser parser) {
-		return SingleConfigurationSource.parse(context, parser);
-	}
-	
 	private static class Content {
 		ConfigurationSource source;
 		long lastModified;

test/core/src/lombok/LombokTestSource.java

@@ -42,9 +42,11 @@ import org.junit.Assert;
 import lombok.core.LombokImmutableList;
 import lombok.core.configuration.BubblingConfigurationResolver;
 import lombok.core.configuration.ConfigurationFile;
+import lombok.core.configuration.ConfigurationFileToSource;
 import lombok.core.configuration.ConfigurationParser;
 import lombok.core.configuration.ConfigurationProblemReporter;
 import lombok.core.configuration.ConfigurationResolver;
+import lombok.core.configuration.ConfigurationSource;
 import lombok.core.configuration.SingleConfigurationSource;
 
 public class LombokTestSource {
@@ -240,12 +242,21 @@ public class LombokTestSource {
 		this.skipIdempotent = skipIdempotent;
 		this.unchanged = unchanged;
 		this.platforms = platformLimit == null ? null : Arrays.asList(platformLimit);
+		
 		ConfigurationProblemReporter reporter = new ConfigurationProblemReporter() {
 			@Override public void report(String sourceDescription, String problem, int lineNumber, CharSequence line) {
 				Assert.fail("Problem on directive line: " + problem + " at conf line #" + lineNumber + " (" + line + ")");
 			}
 		};
-		this.configuration = new BubblingConfigurationResolver(Collections.singleton(SingleConfigurationSource.parse(ConfigurationFile.fromCharSequence(file.getAbsoluteFile().getPath(), conf, ConfigurationFile.getLastModifiedOrMissing(file)), new ConfigurationParser(reporter))));
+		final ConfigurationFile configurationFile = ConfigurationFile.fromCharSequence(file.getAbsoluteFile().getPath(), conf, ConfigurationFile.getLastModifiedOrMissing(file));
+		final ConfigurationSource source = SingleConfigurationSource.parse(configurationFile, new ConfigurationParser(reporter));
+		ConfigurationFileToSource sourceFinder = new ConfigurationFileToSource() {
+			@Override public ConfigurationSource parsed(ConfigurationFile fileLocation) {
+				return fileLocation.equals(configurationFile) ? source : null;
+			}
+		};
+		
+		this.configuration = new BubblingConfigurationResolver(configurationFile, sourceFinder);
 		this.formatPreferences = Collections.unmodifiableMap(formats);
 	}