more tests, minor fixes. some findbugs issues addressed.

org.springframework.expression/src/main/java/org/springframework/expression/TypedValue.java

@@ -29,6 +29,8 @@ public class TypedValue {
 
 	private Object value;
 	private TypeDescriptor typeDescriptor;
+	
+	public static final TypedValue NULL_TYPED_VALUE = new TypedValue(null, TypeDescriptor.NULL_TYPE_DESCRIPTOR);
 
 	/**
 	 * Create a TypedValue for a simple object.  The type descriptor is inferred 

org.springframework.expression/src/main/java/org/springframework/expression/spel/ExpressionState.java

@@ -52,9 +52,10 @@ public class ExpressionState {
 		this.relatedContext = context;
 		createVariableScope();
 	}
-
+	
+	// create an empty top level VariableScope
 	private void createVariableScope() {
-		this.variableScopes.add(new VariableScope()); // create an empty top level VariableScope
+		this.variableScopes.add(new VariableScope()); 
 	}
 
 	/**
@@ -64,9 +65,10 @@ public class ExpressionState {
 		if (this.contextObjects.isEmpty()) {
 			TypedValue rootObject = this.relatedContext.getRootObject();
 			if (rootObject == null) {
-				return new TypedValue(rootObject,TypeDescriptor.NULL_TYPE_DESCRIPTOR);
+				return TypedValue.NULL_TYPED_VALUE;
+			} else {
+				return rootObject;
 			}
-			return rootObject;
 		}
 		return this.contextObjects.peek();
 	}
@@ -80,7 +82,12 @@ public class ExpressionState {
 	}
 
 	public TypedValue getRootContextObject() {
-		return this.relatedContext.getRootObject();
+		TypedValue root = this.relatedContext.getRootObject();
+		if (root == null) {
+			return TypedValue.NULL_TYPED_VALUE;
+		} else {
+			return root;
+		}
 	}
 
 	public void setVariable(String name, Object value) {
@@ -89,7 +96,11 @@ public class ExpressionState {
 
 	public TypedValue lookupVariable(String name) {
 		Object value = this.relatedContext.lookupVariable(name);
-		return new TypedValue(value,TypeDescriptor.forObject(value));
+		if (value==null) {
+			return TypedValue.NULL_TYPED_VALUE;
+		} else {
+			return new TypedValue(value,TypeDescriptor.forObject(value));
+		}
 	}
 
 	public TypeComparator getTypeComparator() {
@@ -108,13 +119,10 @@ public class ExpressionState {
 		return this.relatedContext.getTypeConverter().convertValue(value.getValue(), targetTypeDescriptor);
 	}
 
-//	public <T> T convertValue(TypedValue value, Class<T> targetType) throws EvaluationException {
-//		return this.relatedContext.getTypeConverter().convertValue(value, targetType);
-//	}
-
-	/**
+	/*
 	 * A new scope is entered when a function is invoked
 	 */
+	
 	public void enterScope(Map<String, Object> argMap) {
 		this.variableScopes.push(new VariableScope(argMap));
 	}
@@ -167,7 +175,6 @@ public class ExpressionState {
 	 * A new scope is entered when a function is called and it is used to hold the parameters to the function call.  If the names
 	 * of the parameters clash with those in a higher level scope, those in the higher level scope will not be accessible whilst
 	 * the function is executing.  When the function returns the scope is exited.
-	 *
 	 */
 	private static class VariableScope {
 

org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/NullLiteral.java

@@ -17,7 +17,6 @@
 package org.springframework.expression.spel.ast;
 
 import org.antlr.runtime.Token;
-import org.springframework.core.convert.TypeDescriptor;
 import org.springframework.expression.TypedValue;
 
 /**
@@ -26,20 +25,18 @@ import org.springframework.expression.TypedValue;
  */
 public class NullLiteral extends Literal {
 
-    private static final TypedValue NULL_TYPED_VALUE = new TypedValue(null,TypeDescriptor.NULL_TYPE_DESCRIPTOR);
-    
 	public NullLiteral(Token payload) {
 		super(payload);
 	}
 
 	@Override
 	public TypedValue getLiteralValue() {
-		return NULL_TYPED_VALUE;
+		return TypedValue.NULL_TYPED_VALUE;
 	}
 
 	@Override
 	public String toString() {
-		return null;
+		return "null";
 	}
 
 }

org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/Projection.java

@@ -48,7 +48,7 @@ public class Projection extends SpelNodeImpl {
 		TypedValue op = state.getActiveContextObject();
 
 		Object operand = op.getValue();
-		TypeDescriptor operandTypeDescriptor = op.getTypeDescriptor();
+		// TypeDescriptor operandTypeDescriptor = op.getTypeDescriptor();
 		
 		// When the input is a map, we push a special context object on the stack
 		// before calling the specified operation. This special context object
@@ -58,9 +58,10 @@ public class Projection extends SpelNodeImpl {
 		if (operand instanceof Map) {
 			Map<?, ?> mapdata = (Map<?, ?>) operand;
 			List<Object> result = new ArrayList<Object>();
-			for (Object k : mapdata.keySet()) {
+			mapdata.entrySet();
+			for (Map.Entry entry : mapdata.entrySet()) {
 				try {
-					state.pushActiveContextObject(new TypedValue(new KeyValuePair(k, mapdata.get(k)),TypeDescriptor.valueOf(KeyValuePair.class)));
+					state.pushActiveContextObject(new TypedValue(new KeyValuePair(entry.getKey(), entry.getValue()),TypeDescriptor.valueOf(KeyValuePair.class)));
 					result.add(getChild(0).getValueInternal(state).getValue());
 				} finally {
 					state.popActiveContextObject();

org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/Selection.java

@@ -63,10 +63,10 @@ public class Selection extends SpelNodeImpl {
 			// TODO don't lose generic info for the new map
 			Map<Object,Object> result = new HashMap<Object,Object>();
 			Object lastKey = null;
-			for (Object k : mapdata.keySet()) {
+			for (Map.Entry entry : mapdata.entrySet()) {
 				try {
-					lastKey = k;
-					KeyValuePair kvp = new KeyValuePair(k,mapdata.get(k));
+					lastKey = entry.getKey();
+					KeyValuePair kvp = new KeyValuePair(entry.getKey(),entry.getValue());
 					TypedValue kvpair = new TypedValue(kvp,TypeDescriptor.valueOf(KeyValuePair.class));
 					state.pushActiveContextObject(kvpair);
 					Object o = selectionCriteria.getValueInternal(state).getValue();
@@ -90,9 +90,9 @@ public class Selection extends SpelNodeImpl {
 				return new TypedValue(null,TypeDescriptor.NULL_TYPE_DESCRIPTOR);
 			}
 			if (variant == LAST) {
-				Object lastValue = result.get(lastKey);
 				Map resultMap = new HashMap();
-				resultMap.put(lastKey,result.get(lastKey));
+				Object lastValue = result.get(lastKey);
+				resultMap.put(lastKey,lastValue);
 				return new TypedValue(resultMap,TypeDescriptor.valueOf(Map.class));
 			}
 			return new TypedValue(result,op.getTypeDescriptor());

org.springframework.expression/src/main/java/org/springframework/expression/spel/support/ReflectiveConstructorExecutor.java

@@ -57,8 +57,7 @@ class ReflectiveConstructorExecutor implements ConstructorExecutor {
 				c.setAccessible(true);
 			}
 			return new TypedValue(c.newInstance(arguments),TypeDescriptor.valueOf(c.getClass()));
-		}
-		catch (Exception ex) {
+		} catch (Exception ex) {
 			throw new AccessException("Problem invoking constructor: " + c, ex);
 		}
 	}

org.springframework.expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyResolver.java

@@ -333,6 +333,9 @@ public class ReflectivePropertyResolver implements PropertyAccessor {
 			if (this == other) {
 				return true;
 			}
+			if (!(other instanceof CacheKey)) {
+				return false;
+			}
 			CacheKey otherKey = (CacheKey) other;
 			return (this.clazz.equals(otherKey.clazz) && this.name.equals(otherKey.name));
 		}

org.springframework.expression/src/main/java/org/springframework/expression/spel/support/StandardTypeComparator.java

@@ -34,9 +34,8 @@ public class StandardTypeComparator implements TypeComparator {
 		// If one is null, check if the other is
 		if (left == null) {
 			return right == null ? 0 : 1;
-		}
-		else if (right == null) {
-			return left == null ? 0 : -1;
+		} else if (right == null) {
+			return -1; // left cannot be null
 		}
 
 		// Basic number comparisons