Permalink
Browse files

Better type error messages

Renamed iterable to collection
  • Loading branch information...
1 parent a519eb0 commit e84f6437b5d1979247346eb243286ed562c41c71 @systay committed Aug 22, 2012
Showing with 146 additions and 103 deletions.
  1. +2 −2 cypher/src/docs/dev/ql/functions/index.txt
  2. +1 −1 cypher/src/main/scala/org/neo4j/cypher/CypherException.scala
  3. +1 −1 cypher/src/main/scala/org/neo4j/cypher/internal/commands/AggregationExpression.scala
  4. +3 −1 cypher/src/main/scala/org/neo4j/cypher/internal/commands/Expression.scala
  5. +10 −10 cypher/src/main/scala/org/neo4j/cypher/internal/commands/Functions.scala
  6. +8 −8 cypher/src/main/scala/org/neo4j/cypher/internal/commands/InIterable.scala
  7. +1 −1 cypher/src/main/scala/org/neo4j/cypher/internal/commands/MathFunction.scala
  8. +2 −2 cypher/src/main/scala/org/neo4j/cypher/internal/executionplan/builders/MatchBuilder.scala
  9. +5 −5 cypher/src/main/scala/org/neo4j/cypher/internal/mutation/ForeachAction.scala
  10. +1 −1 cypher/src/main/scala/org/neo4j/cypher/internal/parser/v1_6/Predicates.scala
  11. +1 −1 cypher/src/main/scala/org/neo4j/cypher/internal/parser/v1_7/Expressions.scala
  12. +1 −1 cypher/src/main/scala/org/neo4j/cypher/internal/parser/v1_8/Predicates.scala
  13. +2 −2 cypher/src/main/scala/org/neo4j/cypher/internal/parser/v1_8/Updates.scala
  14. +10 −1 cypher/src/main/scala/org/neo4j/cypher/internal/pipes/matching/MatchingContext.scala
  15. +2 −2 cypher/src/main/scala/org/neo4j/cypher/internal/pipes/matching/PatternNode.scala
  16. +2 −2 cypher/src/main/scala/org/neo4j/cypher/internal/pipes/matching/PatternRelationship.scala
  17. +1 −1 cypher/src/main/scala/org/neo4j/cypher/internal/symbols/AnyIterableType.scala
  18. +1 −1 cypher/src/main/scala/org/neo4j/cypher/internal/symbols/AnyType.scala
  19. +3 −1 cypher/src/main/scala/org/neo4j/cypher/internal/symbols/BooleanType.scala
  20. +7 −3 cypher/src/main/scala/org/neo4j/cypher/internal/symbols/{IterableType.scala → CollectionType.scala}
  21. +3 −1 cypher/src/main/scala/org/neo4j/cypher/internal/symbols/DoubleType.scala
  22. +3 −1 cypher/src/main/scala/org/neo4j/cypher/internal/symbols/IntegerType.scala
  23. +3 −1 cypher/src/main/scala/org/neo4j/cypher/internal/symbols/LongType.scala
  24. +3 −1 cypher/src/main/scala/org/neo4j/cypher/internal/symbols/MapType.scala
  25. +3 −1 cypher/src/main/scala/org/neo4j/cypher/internal/symbols/NodeType.scala
  26. +3 −1 cypher/src/main/scala/org/neo4j/cypher/internal/symbols/NumberType.scala
  27. +1 −5 cypher/src/main/scala/org/neo4j/cypher/internal/symbols/PathType.scala
  28. +3 −1 cypher/src/main/scala/org/neo4j/cypher/internal/symbols/RelationshipType.scala
  29. +3 −4 cypher/src/main/scala/org/neo4j/cypher/internal/symbols/ScalarType.scala
  30. +3 −5 cypher/src/main/scala/org/neo4j/cypher/internal/symbols/StringType.scala
  31. +1 −1 cypher/src/main/scala/org/neo4j/cypher/internal/symbols/SymbolTable.scala
  32. +1 −1 cypher/src/test/scala/org/neo4j/cypher/CypherParserTest.scala
  33. +20 −0 cypher/src/test/scala/org/neo4j/cypher/ErrorMessagesTest.scala
  34. +0 −2 cypher/src/test/scala/org/neo4j/cypher/ExecutionEngineTest.scala
  35. +4 −4 cypher/src/test/scala/org/neo4j/cypher/{SematicErrorTest.scala → SemanticErrorTest.scala}
  36. +1 −1 cypher/src/test/scala/org/neo4j/cypher/docgen/CreateTest.scala
  37. +21 −21 cypher/src/test/scala/org/neo4j/cypher/docgen/FunctionsTest.scala
  38. +2 −1 cypher/src/test/scala/org/neo4j/cypher/docgen/MatchTest.scala
  39. +2 −2 cypher/src/test/scala/org/neo4j/cypher/internal/commands/ExtractTest.scala
  40. +2 −2 cypher/src/test/scala/org/neo4j/cypher/internal/symbols/SymbolTableTest.scala
@@ -36,8 +36,8 @@ include::last.txt[]
:leveloffset: 2
-== Iterable functions ==
-Iterable functions return an iterable of things -- nodes in a path, and so on.
+== Collection functions ==
+Collection functions return collections of things -- nodes in a path, and so on.
:leveloffset: 3
@@ -33,7 +33,7 @@ class CypherTypeException(message: String, cause: Throwable = null) extends Cyph
class IterableRequiredException(message:String, cause:Throwable) extends CypherException(message, cause) {
def this(message:String) = this(message, null)
- def this(expression:Expression) = this("Expected " + expression.identifier.name + " to be an iterable, but it is not.", null)
+ def this(expression:Expression) = this("Expected " + expression.identifier.name + " to be a collection, but it is not.", null)
}
class ParameterNotFoundException(message:String, cause:Throwable) extends CypherException(message, cause) {
@@ -153,7 +153,7 @@ case class Avg(anInner: Expression) extends AggregationWithInnerExpression(anInn
}
case class Collect(anInner: Expression) extends AggregationWithInnerExpression(anInner) {
- def typ = new IterableType(anInner.identifier.typ)
+ def typ = new CollectionType(anInner.identifier.typ)
def name = "collect"
@@ -34,7 +34,7 @@ abstract class Expression extends (Map[String, Any] => Any) {
def dependencies(expectedType: AnyType): Seq[Identifier] = {
val myType = identifier.typ
if (!expectedType.isAssignableFrom(myType))
- throw new SyntaxException(identifier.name + " expected to be of type " + expectedType + " but it is of type " + identifier.typ)
+ throw new SyntaxException("`%s` expected to be a %s but it is a %s".format(identifier.name, expectedType, identifier.typ))
declareDependencies(expectedType)
}
@@ -238,7 +238,9 @@ case class Entity(entityName: String) extends CastableExpression {
case class Collection(expressions:Expression*) extends CastableExpression {
def compute(m: Map[String, Any]): Any = expressions.map(e=>e(m))
+
val identifier: Identifier = Identifier(name, AnyIterableType())
+
private def name = expressions.map(_.identifier.name).mkString("[", ", ", "]")
def declareDependencies(extectedType: AnyType): Seq[Identifier] = expressions.flatMap(_.declareDependencies(AnyType()))
def rewrite(f: (Expression) => Expression): Expression = f(Collection(expressions.map(f):_*))
@@ -36,28 +36,28 @@ abstract class NullInNullOutExpression(argument: Expression) extends Expression
}
}
-case class ExtractFunction(iterable: Expression, id: String, expression: Expression)
- extends NullInNullOutExpression(iterable)
+case class ExtractFunction(collection: Expression, id: String, expression: Expression)
+ extends NullInNullOutExpression(collection)
with IterableSupport
{
def compute(value: Any, m: Map[String, Any]) = makeTraversable(value).map(iterValue => {
val innerMap = m + (id -> iterValue)
expression(innerMap)
}).toList
- val identifier = Identifier("extract(" + id + " in " + iterable.identifier.name + " : " + expression.identifier.name + ")", new IterableType(expression.identifier.typ))
+ val identifier = Identifier("extract(" + id + " in " + collection.identifier.name + " : " + expression.identifier.name + ")", new CollectionType(expression.identifier.typ))
def declareDependencies(extectedType: AnyType): Seq[Identifier] =
// Extract depends on everything that the iterable and the expression depends on, except
// the new identifier inserted into the expression context, named with id
- iterable.dependencies(AnyIterableType()) ++ expression.dependencies(AnyType()).filterNot(_.name == id)
+ collection.dependencies(AnyIterableType()) ++ expression.dependencies(AnyType()).filterNot(_.name == id)
- def rewrite(f: (Expression) => Expression) = f(ExtractFunction(iterable.rewrite(f), id, expression.rewrite(f)))
+ def rewrite(f: (Expression) => Expression) = f(ExtractFunction(collection.rewrite(f), id, expression.rewrite(f)))
def filter(f: (Expression) => Boolean) = if (f(this))
- Seq(this) ++ iterable.filter(f) ++ expression.filter(f)
+ Seq(this) ++ collection.filter(f) ++ expression.filter(f)
else
- iterable.filter(f) ++ expression.filter(f)
+ collection.filter(f) ++ expression.filter(f)
}
case class RelationshipFunction(path: Expression) extends NullInNullOutExpression(path) {
@@ -66,7 +66,7 @@ case class RelationshipFunction(path: Expression) extends NullInNullOutExpressio
case x => throw new SyntaxException("Expected " + path.identifier.name + " to be a path.")
}
- val identifier = Identifier("RELATIONSHIPS(" + path.identifier.name + ")", new IterableType(RelationshipType()))
+ val identifier = Identifier("RELATIONSHIPS(" + path.identifier.name + ")", new CollectionType(RelationshipType()))
def declareDependencies(extectedType: AnyType): Seq[Identifier] = path.dependencies(PathType())
@@ -160,7 +160,7 @@ case class HeadFunction(collection: Expression) extends NullInNullOutExpression(
def compute(value: Any, m: Map[String, Any]) = makeTraversable(value).head
private def myType = collection.identifier.typ match {
- case x: IterableType => x.iteratedType
+ case x: CollectionType => x.iteratedType
case _ => ScalarType()
}
@@ -214,7 +214,7 @@ case class NodesFunction(path: Expression) extends NullInNullOutExpression(path)
case x => throw new SyntaxException("Expected " + path.identifier.name + " to be a path.")
}
- val identifier = Identifier("NODES(" + path.identifier.name + ")", new IterableType(NodeType()))
+ val identifier = Identifier("NODES(" + path.identifier.name + ")", new CollectionType(NodeType()))
def declareDependencies(extectedType: AnyType): Seq[Identifier] = path.dependencies(PathType())
@@ -54,36 +54,36 @@ abstract class InIterable(expression: Expression, symbol: String, closure: Predi
def filter(f: (Expression) => Boolean): Seq[Expression] = expression.filter(f) ++ closure.filter(f)
}
-case class AllInIterable(iterable: Expression, symbolName: String, inner: Predicate) extends InIterable(iterable, symbolName, inner) {
+case class AllInIterable(collection: Expression, symbolName: String, inner: Predicate) extends InIterable(collection, symbolName, inner) {
def seqMethod[U](f: Seq[U]): ((U) => Boolean) => Boolean = f.forall _
def name = "all"
- def rewrite(f: (Expression) => Expression) = AllInIterable(iterable.rewrite(f), symbolName, inner.rewrite(f))
+ def rewrite(f: (Expression) => Expression) = AllInIterable(collection.rewrite(f), symbolName, inner.rewrite(f))
}
-case class AnyInIterable(iterable: Expression, symbolName: String, inner: Predicate) extends InIterable(iterable, symbolName, inner) {
+case class AnyInIterable(collection: Expression, symbolName: String, inner: Predicate) extends InIterable(collection, symbolName, inner) {
def seqMethod[U](f: Seq[U]): ((U) => Boolean) => Boolean = f.exists _
def name = "any"
- def rewrite(f: (Expression) => Expression) = AnyInIterable(iterable.rewrite(f), symbolName, inner.rewrite(f))
+ def rewrite(f: (Expression) => Expression) = AnyInIterable(collection.rewrite(f), symbolName, inner.rewrite(f))
}
-case class NoneInIterable(iterable: Expression, symbolName: String, inner: Predicate) extends InIterable(iterable, symbolName, inner) {
+case class NoneInIterable(collection: Expression, symbolName: String, inner: Predicate) extends InIterable(collection, symbolName, inner) {
def seqMethod[U](f: Seq[U]): ((U) => Boolean) => Boolean = x => !f.exists(x)
def name = "none"
- def rewrite(f: (Expression) => Expression) = NoneInIterable(iterable.rewrite(f), symbolName, inner.rewrite(f))
+ def rewrite(f: (Expression) => Expression) = NoneInIterable(collection.rewrite(f), symbolName, inner.rewrite(f))
}
-case class SingleInIterable(iterable: Expression, symbolName: String, inner: Predicate) extends InIterable(iterable, symbolName, inner) {
+case class SingleInIterable(collection: Expression, symbolName: String, inner: Predicate) extends InIterable(collection, symbolName, inner) {
def seqMethod[U](f: Seq[U]): ((U) => Boolean) => Boolean = x => f.filter(x).length == 1
def name = "single"
- def rewrite(f: (Expression) => Expression) = SingleInIterable(iterable.rewrite(f), symbolName, inner.rewrite(f))
+ def rewrite(f: (Expression) => Expression) = SingleInIterable(collection.rewrite(f), symbolName, inner.rewrite(f))
}
object IsIterable extends IterableSupport {
@@ -85,7 +85,7 @@ case class RangeFunction(start: Expression, end: Expression, step: Expression) e
}
}
- val identifier = Identifier("range("+ start + "," + end + "," + step + ")", new IterableType(NumberType()))
+ val identifier = Identifier("range("+ start + "," + end + "," + step + ")", new CollectionType(NumberType()))
def rewrite(f: (Expression) => Expression) = f(RangeFunction(start.rewrite(f), end.rewrite(f), step.rewrite(f)))
}
@@ -99,10 +99,10 @@ trait PatternGraphBuilder {
patternRelMap(rel) = leftNode.relateTo(rel, rightNode, relType, dir, optional, predicate)
}
- case VarLengthRelatedTo(pathName, start, end, minHops, maxHops, relType, dir, iterableRel, optional, predicate) => {
+ case VarLengthRelatedTo(pathName, start, end, minHops, maxHops, relType, dir, relsCollection, optional, predicate) => {
val startNode: PatternNode = patternNodeMap.getOrElseUpdate(start, new PatternNode(start))
val endNode: PatternNode = patternNodeMap.getOrElseUpdate(end, new PatternNode(end))
- patternRelMap(pathName) = startNode.relateViaVariableLengthPathTo(pathName, endNode, minHops, maxHops, relType, dir, iterableRel, optional, predicate)
+ patternRelMap(pathName) = startNode.relateViaVariableLengthPathTo(pathName, endNode, minHops, maxHops, relType, dir, relsCollection, optional, predicate)
}
case _ =>
})
@@ -24,7 +24,7 @@ import org.neo4j.cypher.internal.symbols.AnyIterableType
import org.neo4j.cypher.internal.pipes.{QueryState, ExecutionContext}
-case class ForeachAction(iterable: Expression, symbol: String, actions: Seq[UpdateAction])
+case class ForeachAction(collection: Expression, symbol: String, actions: Seq[UpdateAction])
extends UpdateAction
with IterableSupport {
def dependencies = {
@@ -34,13 +34,13 @@ case class ForeachAction(iterable: Expression, symbol: String, actions: Seq[Upda
filterNot(_.name == symbol). //remove dependencies to the symbol we're introducing
filterNot(ownIdentifiers contains) //remove dependencies to identifiers we are introducing
- iterable.dependencies(AnyIterableType()) ++ updateDeps
+ collection.dependencies(AnyIterableType()) ++ updateDeps
}
def exec(context: ExecutionContext, state: QueryState) = {
val before = context.get(symbol)
- val seq = makeTraversable(iterable(context))
+ val seq = makeTraversable(collection(context))
seq.foreach(element => {
context.put(symbol, element)
@@ -59,9 +59,9 @@ case class ForeachAction(iterable: Expression, symbol: String, actions: Seq[Upda
Stream(context)
}
- def filter(f: (Expression) => Boolean) = Some(iterable).filter(f).toSeq ++ actions.flatMap(_.filter(f))
+ def filter(f: (Expression) => Boolean) = Some(collection).filter(f).toSeq ++ actions.flatMap(_.filter(f))
- def rewrite(f: (Expression) => Expression) = ForeachAction(f(iterable), symbol, actions.map(_.rewrite(f)))
+ def rewrite(f: (Expression) => Expression) = ForeachAction(f(collection), symbol, actions.map(_.rewrite(f)))
def identifier = Seq.empty
}
@@ -57,7 +57,7 @@ trait Predicates extends Base with Expressions with ReturnItems {
def sequencePredicate: Parser[Predicate] = allInSeq | anyInSeq | noneInSeq | singleInSeq
def symbolIterablePredicate: Parser[(Expression, String, Predicate)] =
- (identity ~ ignoreCase("in") ~ expression ~ ignoreCase("where") ~ predicate ^^ { case symbol ~ in ~ iterable ~ where ~ klas => (iterable, symbol, klas) }
+ (identity ~ ignoreCase("in") ~ expression ~ ignoreCase("where") ~ predicate ^^ { case symbol ~ in ~ collection ~ where ~ klas => (collection, symbol, klas) }
|identity ~> ignoreCase("in") ~ expression ~> failure("expected where"))
@@ -181,7 +181,7 @@ trait Expressions extends Base {
def sequencePredicate: Parser[Predicate] = allInSeq | anyInSeq | noneInSeq | singleInSeq | in
def symbolIterablePredicate: Parser[(Expression, String, Predicate)] =
- (identity ~ ignoreCase("in") ~ expression ~ ignoreCase("where") ~ predicate ^^ { case symbol ~ in ~ iterable ~ where ~ klas => (iterable, symbol, klas) }
+ (identity ~ ignoreCase("in") ~ expression ~ ignoreCase("where") ~ predicate ^^ { case symbol ~ in ~ collection ~ where ~ klas => (collection, symbol, klas) }
|identity ~> ignoreCase("in") ~ expression ~> failure("expected where"))
def in : Parser[Predicate] = expression ~ ignoreCase("in") ~ expression ^^ {
@@ -47,7 +47,7 @@ trait Predicates extends Base with ParserPattern with StringLiteral {
def sequencePredicate: Parser[Predicate] = allInSeq | anyInSeq | noneInSeq | singleInSeq | in
def symbolIterablePredicate: Parser[(Expression, String, Predicate)] =
- (identity ~ ignoreCase("in") ~ expression ~ ignoreCase("where") ~ predicate ^^ { case symbol ~ in ~ iterable ~ where ~ klas => (iterable, symbol, klas) }
+ (identity ~ ignoreCase("in") ~ expression ~ ignoreCase("where") ~ predicate ^^ { case symbol ~ in ~ collection ~ where ~ klas => (collection, symbol, klas) }
|identity ~> ignoreCase("in") ~ expression ~> failure("expected where"))
def in: Parser[Predicate] = expression ~ ignoreCase("in") ~ expression ^^ {
@@ -27,13 +27,13 @@ trait Updates extends Base with Expressions with StartClause {
def updates: Parser[(Seq[UpdateAction], Seq[NamedPath])] = rep(delete | set | foreach) ^^ (cmds => reduce(cmds))
def foreach: Parser[(Seq[UpdateAction], Seq[NamedPath])] = ignoreCase("foreach") ~> "(" ~> identity ~ ignoreCase("in") ~ expression ~ ":" ~ opt(createStart) ~ opt(updates) <~ ")" ^^ {
- case id ~ in ~ iterable ~ ":" ~ creates ~ innerUpdates => {
+ case id ~ in ~ collection ~ ":" ~ creates ~ innerUpdates => {
val createCmds = creates.toSeq.map(_._1.map(_.asInstanceOf[UpdateAction])).flatten
val reducedItems: (Seq[UpdateAction], Seq[NamedPath]) = reduce(innerUpdates.toSeq)
val updateCmds = reducedItems._1
val namedPaths = reducedItems._2 ++ creates.toSeq.flatMap(_._2)
if(namedPaths.nonEmpty) throw new SyntaxException("Paths can't be created inside of foreach")
- (Seq(ForeachAction(iterable, id, createCmds ++ updateCmds)), Seq())
+ (Seq(ForeachAction(collection, id, createCmds ++ updateCmds)), Seq())
}
}
@@ -38,7 +38,16 @@ class MatchingContext(boundIdentifiers: SymbolTable,
private def identifiers:Seq[Identifier] = patternGraph.patternRels.values.flatMap(p => p.identifiers).toSeq
- lazy val symbols = boundIdentifiers.add(identifiers: _*)
+ lazy val symbols = {
+ val ids = identifiers
+
+ val identifiersAlreadyInContext = ids.filter(identifier => boundIdentifiers.keys.contains(identifier.name))
+
+ identifiersAlreadyInContext.foreach( boundIdentifiers.assertHas )
+
+ boundIdentifiers.keys.filter(_)
+ boundIdentifiers.add(ids: _*)
+ }
def getMatches(sourceRow: Map[String, Any]): Traversable[Map[String, Any]] = {
builder.getMatches(sourceRow)
@@ -43,10 +43,10 @@ class PatternNode(key: String) extends PatternElement(key) {
maxHops: Option[Int],
relType: Seq[String],
dir: Direction,
- iterableRel: Option[String],
+ collectionOfRels: Option[String],
optional: Boolean,
predicate: Predicate): PatternRelationship = {
- val rel = new VariableLengthPatternRelationship(pathName, this, end, iterableRel, minHops, maxHops, relType, dir, optional, predicate)
+ val rel = new VariableLengthPatternRelationship(pathName, this, end, collectionOfRels, minHops, maxHops, relType, dir, optional, predicate)
relationships.add(rel)
end.relationships.add(rel)
rel
@@ -119,8 +119,8 @@ class VariableLengthPatternRelationship(pathName: String,
override def identifiers : Seq[Identifier] = Seq(
Identifier(startNode.key, NodeType()),
Identifier(endNode.key, NodeType()),
- Identifier(key, new IterableType(RelationshipType()))) ++
- relIterable.toSeq.map(Identifier(_, new IterableType(RelationshipType())))
+ Identifier(key, new CollectionType(RelationshipType()))) ++
+ relIterable.toSeq.map(Identifier(_, new CollectionType(RelationshipType())))
override def getGraphRelationships(node: PatternNode, realNode: Node): Seq[GraphRelationship] = {
@@ -20,6 +20,6 @@
package org.neo4j.cypher.internal.symbols
object AnyIterableType {
- val instance = new IterableType(AnyType())
+ val instance = new CollectionType(AnyType())
def apply() = instance
}
@@ -55,7 +55,7 @@ class AnyType {
def isAssignableFrom(other: AnyType): Boolean = this.getClass.isAssignableFrom(other.getClass)
- override def toString: String = this.getClass.getSimpleName
+ override def toString: String = "Any"
}
@@ -25,4 +25,6 @@ object BooleanType {
def apply() = instance
}
-class BooleanType extends ScalarType
+class BooleanType extends ScalarType {
+ override def toString = "Boolean"
+}
@@ -22,11 +22,15 @@ package org.neo4j.cypher.internal.symbols
import java.lang.String
-class IterableType(val iteratedType: AnyType) extends AnyType {
+class CollectionType(val iteratedType: AnyType) extends AnyType {
- override def toString: String = "IterableType<" + iteratedType + ">"
+ override def toString: String =
+ if (iteratedType.isInstanceOf[AnyType])
+ "Collection"
+ else
+ "Collection<" + iteratedType + ">"
- override def isAssignableFrom(other:AnyType):Boolean = super.isAssignableFrom(other) && iteratedType.isAssignableFrom(other.asInstanceOf[IterableType].iteratedType)
+ override def isAssignableFrom(other: AnyType): Boolean = super.isAssignableFrom(other) && iteratedType.isAssignableFrom(other.asInstanceOf[CollectionType].iteratedType)
}
@@ -25,7 +25,9 @@ object DoubleType {
def apply() = instance
}
-class DoubleType extends NumberType
+class DoubleType extends NumberType {
+ override def toString = "Double"
+}
@@ -26,7 +26,9 @@ object IntegerType {
def apply() = instance
}
-class IntegerType extends NumberType
+class IntegerType extends NumberType {
+ override def toString = "Integer"
+}
@@ -25,7 +25,9 @@ object LongType {
def apply() = instance
}
-class LongType extends NumberType
+class LongType extends NumberType {
+ override def toString = "Long"
+}
@@ -26,7 +26,9 @@ object MapType {
}
-class MapType extends AnyType
+class MapType extends AnyType {
+ override def toString = "Map"
+}
Oops, something went wrong.

0 comments on commit e84f643

Please sign in to comment.