feature: add SourcePosition#isValidPosition(), to detect invalid posi…

…tion without null or -1 checks (#1964)
INRIA · Apr 17, 2018 · 7f29a81 · 7f29a81
1 parent 4e15b94
commit 7f29a81
Show file tree

Hide file tree

Showing 31 changed files with 119 additions and 105 deletions.
diff --git a/src/main/java/spoon/reflect/cu/CompilationUnit.java b/src/main/java/spoon/reflect/cu/CompilationUnit.java
@@ -23,14 +23,15 @@
 import spoon.reflect.declaration.CtImport;
 
 import java.io.File;
+import java.io.Serializable;
 import java.util.Collection;
 import java.util.List;
 
 /**
  * Defines a compilation unit. In Java, a compilation unit can contain only one
  * public type declaration and other secondary types declarations (not public).
  */
-public interface CompilationUnit extends FactoryAccessor {
+public interface CompilationUnit extends FactoryAccessor, Serializable {
 
 	enum UNIT_TYPE {
 		TYPE_DECLARATION,

diff --git a/src/main/java/spoon/reflect/cu/SourcePosition.java b/src/main/java/spoon/reflect/cu/SourcePosition.java
@@ -29,6 +29,12 @@ public interface SourcePosition extends Serializable {
 
 	SourcePosition NOPOSITION = new NoSourcePosition();
 
+	/**
+	 * @return true if this instance holds start/end indexes of related sources.
+	 * false if they are unknown
+	 */
+	boolean isValidPosition();
+
 	/**
 	 * Returns a string representation of this position in the form
 	 * "sourcefile:line", or "sourcefile" if no line number is available.

diff --git a/src/main/java/spoon/reflect/cu/position/NoSourcePosition.java b/src/main/java/spoon/reflect/cu/position/NoSourcePosition.java
@@ -23,7 +23,7 @@
 import java.io.Serializable;
 
 /**
- * This interface represents the position of a program element in a source file.
+ * This class represents the position of a program element in a source file.
  */
 public class NoSourcePosition implements SourcePosition, Serializable {
 
@@ -39,34 +39,39 @@ public CompilationUnit getCompilationUnit() {
 		return null;
 	}
 
+	@Override
+	public boolean isValidPosition() {
+		return false;
+	}
+
 	@Override
 	public int getLine() {
-		return -1;
+		throw new UnsupportedOperationException("PartialSourcePosition only contains a CompilationUnit");
 	}
 
 	@Override
 	public int getEndLine() {
-		return -1;
+		throw new UnsupportedOperationException("PartialSourcePosition only contains a CompilationUnit");
 	}
 
 	@Override
 	public int getColumn() {
-		return -1;
+		throw new UnsupportedOperationException("PartialSourcePosition only contains a CompilationUnit");
 	}
 
 	@Override
 	public int getEndColumn() {
-		return -1;
+		throw new UnsupportedOperationException("PartialSourcePosition only contains a CompilationUnit");
 	}
 
 	@Override
 	public int getSourceEnd() {
-		return -1;
+		throw new UnsupportedOperationException("PartialSourcePosition only contains a CompilationUnit");
 	}
 
 	@Override
 	public int getSourceStart() {
-		return -1;
+		throw new UnsupportedOperationException("PartialSourcePosition only contains a CompilationUnit");
 	}
 
 	@Override

diff --git a/src/main/java/spoon/reflect/declaration/CtElement.java b/src/main/java/spoon/reflect/declaration/CtElement.java
@@ -126,7 +126,9 @@ <A extends Annotation> CtAnnotation<A> getAnnotation(
 	/**
 	 * Gets the position of this element in input source files
 	 *
-	 * @return Source file and line number of this element or null
+	 * @return Source file and line number of this element.
+	 * It never returns null. Use {@link SourcePosition#isValidPosition()}
+	 * to detect whether return instance contains start/end indexes.
 	 */
 	@PropertyGetter(role = POSITION)
 	SourcePosition getPosition();

diff --git a/src/main/java/spoon/reflect/factory/CompilationUnitFactory.java b/src/main/java/spoon/reflect/factory/CompilationUnitFactory.java
@@ -60,7 +60,7 @@ public CompilationUnit create() {
 	}
 
 	public CompilationUnit getOrCreate(CtPackage ctPackage) {
-		if (ctPackage.getPosition() != null && ctPackage.getPosition().getCompilationUnit() != null) {
+		if (ctPackage.getPosition().getCompilationUnit() != null) {
 			return ctPackage.getPosition().getCompilationUnit();
 		} else {
 
@@ -88,7 +88,7 @@ public CompilationUnit getOrCreate(CtType type) {
 		if (type == null) {
 			return null;
 		}
-		if (type.getPosition() != null && type.getPosition().getCompilationUnit() != null) {
+		if (type.getPosition().getCompilationUnit() != null) {
 			return type.getPosition().getCompilationUnit();
 		}
 
@@ -117,7 +117,7 @@ public CompilationUnit getOrCreate(CtType type) {
 	}
 
 	public CompilationUnit getOrCreate(CtModule module) {
-		if (module.getPosition() != null && module.getPosition().getCompilationUnit() != null) {
+		if (module.getPosition().getCompilationUnit() != null) {
 			return module.getPosition().getCompilationUnit();
 		} else {
 			File file = this.factory.getEnvironment().getOutputDestinationHandler().getOutputPath(module, null, null).toFile();

diff --git a/src/main/java/spoon/reflect/visitor/AccessibleVariablesFinder.java b/src/main/java/spoon/reflect/visitor/AccessibleVariablesFinder.java
@@ -68,9 +68,7 @@ class VariableScanner extends CtInheritanceScanner {
 			public void visitCtStatementList(CtStatementList e) {
 				for (int i = 0; i < e.getStatements().size(); i++) {
 					CtStatement ctStatement = e.getStatements().get(i);
-					if (ctStatement.getPosition() == null) {
-					}
-					if (ctStatement.getPosition() != null
+					if (ctStatement.getPosition().isValidPosition()
 							&& ctStatement.getPosition().getSourceStart() > expression.getPosition().getSourceEnd()) {
 						break;
 					}

diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java
@@ -346,7 +346,7 @@ public DefaultJavaPrettyPrinter scan(CtElement e) {
 			} catch (Exception ex) {
 				String elementInfo = e.getClass().getName();
 				elementInfo += " on path " + getPath(e) + "\n";
-				if (e.getPosition() != null) {
+				if (e.getPosition().isValidPosition()) {
 					elementInfo += "at position " + e.getPosition().toString() + " ";
 				}
 				throw new SpoonException("Printing of " + elementInfo + "failed", ex);
@@ -1196,7 +1196,7 @@ public void visitCtIf(CtIf ifElement) {
 			List<CtComment> comments = elementPrinterHelper.getComments(ifElement, CommentOffset.INSIDE);
 			for (CtComment comment : comments) {
 				SourcePosition thenPosition =
-						ifElement.getThenStatement().getPosition() == null ? ((CtBlock) ifElement.getThenStatement()).getStatement(0).getPosition() : ifElement.getThenStatement().getPosition();
+						ifElement.getThenStatement().getPosition().isValidPosition() ? ifElement.getThenStatement().getPosition() : ((CtBlock) ifElement.getThenStatement()).getStatement(0).getPosition();
 				if (comment.getPosition().getSourceStart() > thenPosition.getSourceEnd()) {
 					elementPrinterHelper.writeComment(comment);
 				}
@@ -1343,7 +1343,7 @@ public <T> void visitCtMethod(CtMethod<T> m) {
 		if (m.getBody() != null) {
 			printer.writeSpace();
 			scan(m.getBody());
-			if (m.getBody().getPosition() != null) {
+			if (m.getBody().getPosition().isValidPosition()) {
 				if (m.getBody().getPosition().getCompilationUnit() == sourceCompilationUnit) {
 					if (m.getBody().getStatements().isEmpty() || !(m.getBody().getStatements().get(m.getBody().getStatements().size() - 1) instanceof CtReturn)) {
 						getPrinterHelper().putLineNumberMapping(m.getBody().getPosition().getEndLine());

diff --git a/src/main/java/spoon/reflect/visitor/ElementPrinterHelper.java b/src/main/java/spoon/reflect/visitor/ElementPrinterHelper.java
@@ -414,7 +414,7 @@ public List<CtComment> getComments(CtElement element, CommentOffset offset) {
 			if (comment.getCommentType() == CtComment.CommentType.FILE) {
 				continue;
 			}
-			if (comment.getPosition() == null || element.getPosition() == null) {
+			if (comment.getPosition().isValidPosition() == false || element.getPosition().isValidPosition() == false) {
 				if (offset == CommentOffset.BEFORE) {
 					commentsToPrint.add(comment);
 				}

diff --git a/src/main/java/spoon/reflect/visitor/LiteralHelper.java b/src/main/java/spoon/reflect/visitor/LiteralHelper.java
@@ -43,7 +43,7 @@ public static <T> String getLiteralToken(CtLiteral<T> literal) {
 			boolean mayContainsSpecialCharacter = true;
 
 			SourcePosition position = literal.getPosition();
-			if (position != null) {
+			if (position.isValidPosition()) {
 				// the size of the string in the source code, the -1 is the size of the ' or " in the source code
 				int stringLength = position.getSourceEnd() - position.getSourceStart() - 1;
 				// if the string in the source is not the same as the string in the literal, the string may contains special characters
@@ -58,7 +58,7 @@ public static <T> String getLiteralToken(CtLiteral<T> literal) {
 			boolean mayContainsSpecialCharacters = true;
 
 			SourcePosition position = literal.getPosition();
-			if (position != null) {
+			if (position.isValidPosition()) {
 				// the size of the string in the source code, the -1 is the size of the ' or " in the source code
 				int stringLength = position.getSourceEnd() - position.getSourceStart() - 1;
 				// if the string in the source is not the same as the string in the literal, the string may contains special characters

diff --git a/src/main/java/spoon/reflect/visitor/ModelConsistencyChecker.java b/src/main/java/spoon/reflect/visitor/ModelConsistencyChecker.java
@@ -89,7 +89,7 @@ protected void exit(CtElement e) {
 	private void dumpStack() {
 		environment.debugMessage("model consistency checker stack:");
 		for (CtElement e : stack) {
-			environment.debugMessage("    " + e.getClass().getSimpleName() + " " + (e.getPosition() == null ? "(?)" : "" + e.getPosition()));
+			environment.debugMessage("    " + e.getClass().getSimpleName() + " " + (e.getPosition().isValidPosition() ? String.valueOf(e.getPosition()) : "(?)"));
 		}
 	}
 

diff --git a/src/main/java/spoon/reflect/visitor/PrinterHelper.java b/src/main/java/spoon/reflect/visitor/PrinterHelper.java
@@ -19,7 +19,6 @@
 import spoon.compiler.Environment;
 import spoon.reflect.cu.CompilationUnit;
 import spoon.reflect.cu.SourcePosition;
-import spoon.reflect.cu.position.NoSourcePosition;
 import spoon.reflect.declaration.CtElement;
 import spoon.support.reflect.cu.position.PartialSourcePositionImpl;
 
@@ -237,7 +236,7 @@ private boolean isWhite(char c) {
 
 	/** writes as many newlines as needed to align the line number again between the element position and the current line number */
 	public PrinterHelper adjustStartPosition(CtElement e) {
-		if (e.getPosition() != null && !e.isImplicit() && !(e.getPosition() instanceof NoSourcePosition)) {
+		if (!e.isImplicit() && e.getPosition().isValidPosition()) {
 			// we should add some lines
 			while (line < e.getPosition().getLine()) {
 				writeln();
@@ -253,7 +252,7 @@ public PrinterHelper adjustStartPosition(CtElement e) {
 	}
 
 	public PrinterHelper adjustEndPosition(CtElement e) {
-		if (env.isPreserveLineNumbers() && e.getPosition() != null) {
+		if (env.isPreserveLineNumbers() && e.getPosition().isValidPosition()) {
 			// let's add lines if required
 			while (line < e.getPosition().getEndLine()) {
 				writeln();
@@ -270,7 +269,7 @@ public void undefineLine() {
 
 	public void mapLine(CtElement e, CompilationUnit unitExpected) {
 		SourcePosition sp = e.getPosition();
-		if ((sp != null)
+		if ((sp.isValidPosition())
 				&& (sp.getCompilationUnit() == unitExpected)
 				&& (sp instanceof PartialSourcePositionImpl) == false) {
 			// only map elements coming from the source CU

diff --git a/src/main/java/spoon/support/DefaultCoreFactory.java b/src/main/java/spoon/support/DefaultCoreFactory.java
@@ -155,7 +155,6 @@
 import spoon.support.reflect.cu.CompilationUnitImpl;
 import spoon.support.reflect.cu.position.BodyHolderSourcePositionImpl;
 import spoon.support.reflect.cu.position.DeclarationSourcePositionImpl;
-import spoon.support.reflect.cu.position.PartialSourcePositionImpl;
 import spoon.support.reflect.cu.position.SourcePositionImpl;
 import spoon.support.reflect.declaration.CtAnnotationImpl;
 import spoon.support.reflect.declaration.CtAnnotationMethodImpl;
@@ -688,7 +687,7 @@ public SourcePosition createSourcePosition(CompilationUnit compilationUnit, int
 
 	@Override
 	public SourcePosition createPartialSourcePosition(CompilationUnit compilationUnit) {
-		return new PartialSourcePositionImpl(compilationUnit);
+		return ((CompilationUnitImpl) compilationUnit).getOrCreatePartialSourcePosition();
 	}
 
 	@Override

diff --git a/src/main/java/spoon/support/comparator/CtLineElementComparator.java b/src/main/java/spoon/support/comparator/CtLineElementComparator.java
@@ -31,11 +31,11 @@ public class CtLineElementComparator implements Comparator<CtElement>, Serializa
 	 * Reurns -1 if o1 is before o2 in the file
 	 */
 	public int compare(CtElement o1, CtElement o2) {
-		if (o1.getPosition() == null) {
+		if (o1.getPosition().isValidPosition() == false) {
 			return -1;
 		}
 
-		if (o2.getPosition() == null) {
+		if (o2.getPosition().isValidPosition() == false) {
 			// ensures that compare(x,y) = - compare(y,x)
 			return 1;
 		}

diff --git a/src/main/java/spoon/support/comparator/DeepRepresentationComparator.java b/src/main/java/spoon/support/comparator/DeepRepresentationComparator.java
@@ -28,10 +28,10 @@ public class DeepRepresentationComparator implements Comparator<CtElement>, Seri
 
 	@Override
 	public int compare(CtElement o1, CtElement o2) {
-		if (o1.getPosition() == null) {
+		if (o1.getPosition().isValidPosition() == false) {
 			return 1;
 		}
-		if (o2.getPosition() == null) {
+		if (o2.getPosition().isValidPosition() == false) {
 			return -1;
 		}
 		String current = getDeepRepresentation(o1);

diff --git a/src/main/java/spoon/support/compiler/jdt/JDTCommentBuilder.java b/src/main/java/spoon/support/compiler/jdt/JDTCommentBuilder.java
@@ -219,7 +219,7 @@ private CtElement addCommentToNear(final CtComment comment, final Collection<CtE
 		int smallDistance = Integer.MAX_VALUE;
 
 		for (CtElement element : elements) {
-			if (element.getPosition() == null) {
+			if (element.getPosition().isValidPosition() == false) {
 				continue;
 			}
 			if (element.isImplicit()) {
@@ -419,8 +419,8 @@ public void visitCtIf(CtIf e) {
 					}
 				}
 				if (e.getElseStatement() != null) {
-					SourcePosition thenPosition = e.getThenStatement().getPosition() == null ? ((CtBlock) e.getThenStatement()).getStatement(0).getPosition() : e.getThenStatement().getPosition();
-					SourcePosition elsePosition = e.getElseStatement().getPosition() == null ? ((CtBlock) e.getElseStatement()).getStatement(0).getPosition() : e.getElseStatement().getPosition();
+					SourcePosition thenPosition = e.getThenStatement().getPosition().isValidPosition() == false ? ((CtBlock) e.getThenStatement()).getStatement(0).getPosition() : e.getThenStatement().getPosition();
+					SourcePosition elsePosition = e.getElseStatement().getPosition().isValidPosition() == false ? ((CtBlock) e.getElseStatement()).getStatement(0).getPosition() : e.getElseStatement().getPosition();
 					if (comment.getPosition().getSourceStart() > thenPosition.getSourceEnd() && comment.getPosition().getSourceEnd() < elsePosition.getSourceStart()) {
 						e.getElseStatement().addComment(comment);
 					}
@@ -498,7 +498,7 @@ class FindCommentParentScanner extends CtScanner {
 			}
 
 			private boolean isCommentBetweenElementPosition(CtElement element) {
-				return (element.getPosition() != null
+				return (element.getPosition().isValidPosition()
 						&& element.getPosition().getSourceStart() <= this.start
 						&& element.getPosition().getSourceEnd() >= this.end);
 			}
@@ -512,7 +512,7 @@ public void scan(CtElement element) {
 					return;
 				}
 				CtElement body = getBody(element);
-				if (body != null && body.getPosition() == null) {
+				if (body != null && body.getPosition().isValidPosition() == false) {
 					body = null;
 				}
 

diff --git a/src/main/java/spoon/support/compiler/jdt/ParentExiter.java b/src/main/java/spoon/support/compiler/jdt/ParentExiter.java
@@ -786,7 +786,7 @@ public void visitCtPackage(CtPackage ctPackage) {
 				ctPackage.removeType((CtType<?>) child);
 			}
 			ctPackage.addType((CtType<?>) child);
-			if (child.getPosition() != null && child.getPosition().getCompilationUnit() != null) {
+			if (child.getPosition().getCompilationUnit() != null) {
 				CompilationUnit cu = child.getPosition().getCompilationUnit();
 				List<CtType<?>> declaredTypes = new ArrayList<>(cu.getDeclaredTypes());
 				declaredTypes.add((CtType<?>) child);

diff --git a/src/main/java/spoon/support/compiler/jdt/PositionBuilder.java b/src/main/java/spoon/support/compiler/jdt/PositionBuilder.java
@@ -211,7 +211,7 @@ SourcePosition buildPositionCtElement(CtElement e, ASTNode node) {
 
 			sourceEnd = sourceStart + methodDeclaration.selector.length - 1;
 			if (bodyStart == 0) {
-				return SourcePosition.NOPOSITION;
+				return cf.createPartialSourcePosition(cu);
 			}
 			if (e instanceof CtStatementList) {
 				return cf.createSourcePosition(cu, bodyStart - 1, bodyEnd + 1, lineSeparatorPositions);
@@ -262,7 +262,7 @@ private void setModifiersPosition(CtModifiable e, int start, int end) {
 		String modifierContent = String.valueOf(contents, start, end - start + 1);
 		for (CtExtendedModifier modifier: modifiers) {
 			if (modifier.isImplicit()) {
-				modifier.setPosition(SourcePosition.NOPOSITION);
+				modifier.setPosition(cf.createPartialSourcePosition(cu));
 				continue;
 			}
 			int index = modifierContent.indexOf(modifier.getKind().toString());

diff --git a/src/main/java/spoon/support/reflect/cu/CompilationUnitImpl.java b/src/main/java/spoon/support/reflect/cu/CompilationUnitImpl.java
@@ -18,13 +18,15 @@
 
 import spoon.processing.FactoryAccessor;
 import spoon.reflect.cu.CompilationUnit;
+import spoon.reflect.cu.SourcePosition;
 import spoon.reflect.declaration.CtModule;
 import spoon.reflect.declaration.CtPackage;
 import spoon.reflect.declaration.CtType;
 import spoon.reflect.factory.Factory;
 import spoon.reflect.declaration.CtImport;
 import spoon.reflect.visitor.DefaultJavaPrettyPrinter;
 import spoon.reflect.visitor.filter.TypeFilter;
+import spoon.support.reflect.cu.position.PartialSourcePositionImpl;
 
 import java.io.File;
 import java.io.FileInputStream;
@@ -38,6 +40,8 @@
 import static spoon.reflect.ModelElementContainerDefaultCapacities.COMPILATION_UNIT_DECLARED_TYPES_CONTAINER_DEFAULT_CAPACITY;
 
 public class CompilationUnitImpl implements CompilationUnit, FactoryAccessor {
+	private static final long serialVersionUID = 1L;
+
 	Factory factory;
 
 	List<CtType<?>> declaredTypes = new ArrayList<>(COMPILATION_UNIT_DECLARED_TYPES_CONTAINER_DEFAULT_CAPACITY);
@@ -269,6 +273,16 @@ public void setAutoImport(boolean autoImport) {
 		this.autoImport = autoImport;
 	}
 
+	private PartialSourcePositionImpl myPartialSourcePosition;
+	/**
+	 * @return a {@link SourcePosition} which points to this {@link CompilationUnit}. It always returns same value to safe memory.
+	 */
+	public SourcePosition getOrCreatePartialSourcePosition() {
+		if (myPartialSourcePosition == null) {
+			myPartialSourcePosition = new PartialSourcePositionImpl(this);
+		}
+		return myPartialSourcePosition;
+	}
 
 
 }