Permalink
Browse files

Improved pattern error messages

  • Loading branch information...
1 parent 27d8ba5 commit c02a1672dc8025f0fcf13a3c977d6e8f9d991489 @systay committed May 28, 2012
View
4 cypher/src/main/scala/org/neo4j/cypher/internal/parser/v1_8/AbstractPattern.scala
@@ -22,12 +22,14 @@ package org.neo4j.cypher.internal.parser.v1_8
import org.neo4j.graphdb.Direction
import org.neo4j.cypher.internal.commands.{Predicate, Expression}
import collection.Map
+import org.neo4j.cypher.SyntaxException
abstract sealed class AbstractPattern
object PatternWithEnds {
def unapply(p: AbstractPattern): Option[(ParsedEntity, ParsedEntity, Seq[String], Direction, Boolean, Option[Int], Option[String], Predicate)] = p match {
- case ParsedVarLengthRelation(name, _, start, end, typ, dir, optional, predicate, maxDepth, single, relIterator) => Some((start, end, typ, dir, optional, maxDepth, relIterator, predicate))
+ case ParsedVarLengthRelation(name, _, start, end, typ, dir, optional, predicate, None, maxHops, relIterator) => Some((start, end, typ, dir, optional, maxHops, relIterator, predicate))
+ case ParsedVarLengthRelation(_, _, _, _, _, _, _, _, Some(x), _, _) => throw new SyntaxException("Shortest path does not support a minimal length")
case ParsedRelation(name, _, start, end, typ, dir, optional, predicate) => Some((start, end, typ, dir, optional, Some(1), Some(name), predicate))
}
}
View
2 cypher/src/main/scala/org/neo4j/cypher/internal/parser/v1_8/Base.scala
@@ -98,6 +98,8 @@ abstract class Base extends JavaTokenParsers {
def parameter: Parser[Expression] = curly(identity | wholeNumber) ^^ (x => ParameterExpression(x))
override def failure(msg: String): Parser[Nothing] = "" ~> super.failure("INNER" + msg)
+
+ def failure(msg:String, input:Input) = Failure("INNER" + msg, input)
}
class NodeNamer {
var lastNodeNumber = 0
View
4 cypher/src/main/scala/org/neo4j/cypher/internal/parser/v1_8/CypherParserImpl.scala
@@ -44,9 +44,7 @@ with ActualParser {
throw new SyntaxException(message.substring(5), text, input.offset)
else
throw new SyntaxException(message + """
-Unfortunately, you have run into a syntax error that we don't have a nice message for.
-By sending the query that produced this error to cypher@neo4j.org, you'll save the
-puppies and get better error messages in our next release.
+Think we should have better error message here? Help us by sending this query to cypher@neo4j.org.
Thank you, the Neo4j Team.
""", text, input.offset)
View
21 cypher/src/main/scala/org/neo4j/cypher/internal/parser/v1_8/MatchClause.scala
@@ -30,6 +30,13 @@ trait MatchClause extends Base with ParserPattern {
(Match(unnamedPaths: _*), NamedPaths(namedPaths: _*))
}
+ private def successIfEntities[T](l: Expression, r: Expression)(f: (String, String) => T): Maybe[T] = (l, r) match {
+ case (Entity(lName), Entity(rName)) => Yes(f(lName, rName))
+ case (x, Entity(_)) => No("MATCH end points have to be node identifiers - found: " + x)
+ case (Entity(_), x) => No("MATCH end points have to be node identifiers - found: " + x)
+ case (x, y) => No("MATCH end points have to be node identifiers - found: " + x + " and " + y)
+ }
+
private def translate(abstractPattern: AbstractPattern): Maybe[Any] = abstractPattern match {
case ParsedNamedPath(name, patterns) =>
val namedPathPatterns = patterns.map(translate)
@@ -40,15 +47,15 @@ trait MatchClause extends Base with ParserPattern {
case Some(No(msg)) => No(msg)
}
- case ParsedRelation(name, props, ParsedEntity(Entity(left), startProps, True()), ParsedEntity(Entity(right), endProps, True()), relType, dir, optional, predicate) =>
- Yes(RelatedTo(left = left, right = right, relName = name, relTypes = relType, direction = dir, optional = optional, predicate = True()))
+ case ParsedRelation(name, props, ParsedEntity(left, startProps, True()), ParsedEntity(right, endProps, True()), relType, dir, optional, predicate) =>
+ successIfEntities(left, right)((l, r) => RelatedTo(left = l, right = r, relName = name, relTypes = relType, direction = dir, optional = optional, predicate = True()))
- case ParsedVarLengthRelation(name, props, ParsedEntity(Entity(left), startProps, True()), ParsedEntity(Entity(right), endProps, True()), relType, dir, optional, predicate, min, max, relIterator) =>
- Yes(VarLengthRelatedTo(pathName = name, start = left, end = right, minHops = min, maxHops = max, relTypes = relType, direction = dir, relIterator = relIterator, optional = optional, predicate = predicate))
+ case ParsedVarLengthRelation(name, props, ParsedEntity(left, startProps, True()), ParsedEntity(right, endProps, True()), relType, dir, optional, predicate, min, max, relIterator) =>
+ successIfEntities(left, right)((l, r) => VarLengthRelatedTo(pathName = name, start = l, end = r, minHops = min, maxHops = max, relTypes = relType, direction = dir, relIterator = relIterator, optional = optional, predicate = predicate))
- case ParsedShortestPath(name, props, ParsedEntity(Entity(left), startProps, True()), ParsedEntity(Entity(right), endProps, True()), relType, dir, optional, predicate, max, single, relIterator) =>
- Yes(ShortestPath(pathName = name, start = left, end = right, relTypes = relType, dir = dir, maxDepth = max, optional = optional, single = single, relIterator = relIterator, predicate = predicate))
+ case ParsedShortestPath(name, props, ParsedEntity(left, startProps, True()), ParsedEntity(right, endProps, True()), relType, dir, optional, predicate, max, single, relIterator) =>
+ successIfEntities(left, right)((l, r) => ShortestPath(pathName = name, start = l, end = r, relTypes = relType, dir = dir, maxDepth = max, optional = optional, single = single, relIterator = relIterator, predicate = predicate))
- case _ => No("")
+ case x => No("failed to parse MATCH pattern")
}
}
View
26 cypher/src/main/scala/org/neo4j/cypher/internal/parser/v1_8/ParserPattern.scala
@@ -83,7 +83,15 @@ trait ParserPattern extends Base {
case id ~ props => ParsedEntity(Entity(namer.name(id)), props, True())
}
- private def nodeFromExpression = expression ^^ {
+ private def nodeFromExpression = Parser {
+ case in => expression(in) match {
+ case Success(exp, rest) => Success(ParsedEntity(exp, Map[String, Expression](), True()), rest)
+ case x: Error => x
+ case Failure(msg, rest) => failure("expected an expression that is a node", rest)
+ }
+ }
+
+ expression ^^ {
case expression => ParsedEntity(expression, Map[String, Expression](), True())
}
@@ -114,10 +122,9 @@ trait ParserPattern extends Base {
}
}
- private def singleRelationship: Parser[AbstractPattern] =
- onlyOne("expected single path segment", relationship)
+ private def patternForShortestPath: Parser[AbstractPattern] = onlyOne("expected single path segment", relationship)
- private def shortestPath: Parser[List[AbstractPattern]] = (ignoreCase("shortestPath") | ignoreCase("allShortestPaths")) ~ parens(singleRelationship) ^^ {
+ private def shortestPath: Parser[List[AbstractPattern]] = (ignoreCase("shortestPath") | ignoreCase("allShortestPaths")) ~ parens(patternForShortestPath) ^^ {
case algo ~ relInfo =>
val single = algo match {
case "shortestpath" => true
@@ -153,9 +160,15 @@ trait ParserPattern extends Base {
}
}
- private def tailWithRelData = opt("<") ~ "-" ~ "[" ~ opt(identity) ~ opt("?") ~ opt(":" ~> rep1sep(identity, "|")) ~ variable_length ~ props ~ "]" ~ "-" ~ opt(">") ~ node ^^ {
+ private def tailWithRelData: Parser[Tail] = opt("<") ~ "-" ~ "[" ~ opt(identity) ~ opt("?") ~ opt(":" ~> rep1sep(identity, "|")) ~ variable_length ~ props ~ "]" ~ "-" ~ opt(">") ~ node ^^ {
case l ~ "-" ~ "[" ~ rel ~ optional ~ typez ~ varLength ~ properties ~ "]" ~ "-" ~ r ~ end => Tail(direction(l, r), rel, properties, end, varLength, typez.toSeq.flatten.distinct, optional.isDefined)
- }
+ } | linkErrorMessages
+
+ private def linkErrorMessages: Parser[Tail] =
+ opt("<") ~> "-" ~> "[" ~> opt(identity) ~> opt("?") ~> opt(":" ~> rep1sep(identity, "|")) ~> variable_length ~> props ~> "]" ~> failure("expected -") |
+ opt("<") ~> "-" ~> "[" ~> opt(identity) ~> opt("?") ~> opt(":" ~> rep1sep(identity, "|")) ~> variable_length ~> props ~> failure("unclosed bracket") |
+ opt("<") ~> "-" ~> "[" ~> failure("expected relationship information") |
+ opt("<") ~> "-" ~> failure("expected [ or -")
private def variable_length = opt("*" ~ opt(wholeNumber) ~ opt("..") ~ opt(wholeNumber)) ^^ {
case None => None
@@ -218,4 +231,3 @@ trait ParserPattern extends Base {
}
}
-
View
2 cypher/src/main/scala/org/neo4j/cypher/internal/parser/v1_8/StartClause.scala
@@ -26,7 +26,7 @@ import org.neo4j.graphdb.Direction
trait StartClause extends Base with Expressions {
def start: Parser[Start] = createStart | readStart
- def readStart = ignoreCase("start") ~> commaList(startBit) ^^ (x => Start(x: _*))
+ def readStart :Parser[Start] = ignoreCase("start") ~> commaList(startBit) ^^ (x => Start(x: _*)) | failure("expected START or CREATE")
def createStart = ignoreCase("create") ~> commaList(usePattern(translate)) ^^ (x => Start(x.flatten: _*))
View
43 cypher/src/test/scala/org/neo4j/cypher/ErrorMessagesTest.scala
@@ -24,7 +24,6 @@ import org.scalatest.Assertions
import org.junit.Assert._
import org.junit.{Ignore, Test}
-@Ignore
class ErrorMessagesTest extends ExecutionEngineHelper with Assertions with StringExtras {
@Test def noReturnColumns() {
expectError(
@@ -34,16 +33,14 @@ class ErrorMessagesTest extends ExecutionEngineHelper with Assertions with Strin
@Test def badNodeIdentifier() {
expectError(
- "START a = node(0) MATCH a-[WORKED_ON]-[30] return a",
- "expected node identifier")
+ "START a = node(0) MATCH a-[WORKED_ON]-, return a",
+ "expected an expression that is a node")
}
- //TODO Fix this
- @Ignore("Revisit when all start tokens are finished")
@Test def badStart() {
expectError(
"starta = node(0) return a",
- "expected 'START'")
+ "expected START or CREATE")
}
@Test def functionDoesNotExist() {
@@ -84,18 +81,6 @@ class ErrorMessagesTest extends ExecutionEngineHelper with Assertions with Strin
44)
}
- @Test def extraGTSymbol() {
- expectSyntaxError(
- "start p=node(2) match p->[:IS_A]->dude return dude.name",
- "expected [ or -", 24)
- }
-
- @Test def badMatch() {
- expectSyntaxError(
- "start p=node(2) match p-[:IS_A]-!dude return dude.name",
- "expected node identifier", 32)
- }
-
@Test def badMatch2() {
expectSyntaxError(
"start p=node(2) match p-[:IS_A]>dude return dude.name",
@@ -114,17 +99,10 @@ class ErrorMessagesTest extends ExecutionEngineHelper with Assertions with Strin
"expected relationship information", 25)
}
- @Ignore
@Test def badMatch5() {
expectSyntaxError(
"start p=node(2) match p[:likes]->dude return dude.name",
- "`-' expected but `>' found", 24)
- }
-
- @Test def badMatch6() {
- expectSyntaxError(
- "start p=node(2) match p-(:likes)->dude return dude.name",
- "expected [ or -", 24)
+ "failed to parse MATCH pattern", 23)
}
@Test def badMatch7() {
@@ -139,16 +117,14 @@ class ErrorMessagesTest extends ExecutionEngineHelper with Assertions with Strin
"expected [ or -", 24)
}
- @Ignore
- @Test def missingComaBetweenColumns() {
+ @Ignore @Test def missingComaBetweenColumns() {
expectSyntaxError(
"start p=node(2) return sum wo.months",
"Expected comma separated list of returnable values",
22)
}
- @Ignore
- @Test def missingComaBetweenStartNodes() {
+ @Ignore @Test def missingComaBetweenStartNodes() {
expectSyntaxError(
"start a=node(0) b=node(1) return a",
"Expected comma separated list of returnable values",
@@ -196,13 +172,6 @@ class ErrorMessagesTest extends ExecutionEngineHelper with Assertions with Strin
50)
}
- @Test def noPredicatesInWhereClause() {
- expectSyntaxError(
- "START a=node(0) where return a",
- "reserved keyword",
- 29)
- }
-
@Test def functions_and_stuff_have_to_be_renamed_when_sent_through_with() {
expectError(
"START a=node(0) with a, count(*) return a",
View
17 cypher/src/test/scala/org/neo4j/cypher/SyntaxExceptionTest.scala
@@ -21,9 +21,8 @@ package org.neo4j.cypher
import org.scalatest.junit.JUnitSuite
import org.junit.Assert._
-import org.junit.{Ignore, Test}
+import org.junit.{Test}
-@Ignore
class SyntaxExceptionTest extends JUnitSuite {
def expectError(query: String, expectedError: String) {
val parser = new CypherParser()
@@ -73,12 +72,10 @@ class SyntaxExceptionTest extends JUnitSuite {
"Non-mutating queries must return data")
}
- //TODO Fix this
- @Ignore("Revisit when all start tokens are finished")
@Test def shouldWarnAboutMissingStart() {
expectError(
"where s.name = Name and s.age = 10 return s",
- "expected 'START'")
+ "expected START or CREATE")
}
@Test def shouldComplainAboutWholeNumbers() {
@@ -90,13 +87,13 @@ class SyntaxExceptionTest extends JUnitSuite {
@Test def matchWithoutIdentifierHasToHaveParenthesis() {
expectError(
"start a = node(0) match a--b, --> a return a",
- "expected identifier")
+ "expected an expression that is a node")
}
@Test def matchWithoutIdentifierHasToHaveParenthesis2() {
expectError(
"start a = node(0) match (a) -->, a-->b return a",
- "expected node identifier")
+ "expected an expression that is a node")
}
@@ -173,12 +170,6 @@ class SyntaxExceptionTest extends JUnitSuite {
"unknown function")
}
- @Test def nodeParenthesisMustBeClosed() {
- expectError(
- "start s=node(1) match s-->(x return x",
- "Unclosed parenthesis")
- }
-
@Test def handlesMultilineQueries() {
expectError("""start
a=node(0),

0 comments on commit c02a167

Please sign in to comment.