GH3618 - Failed recognition of SQL comments

Author: gliljas

src/NHibernate.Test/EngineTest/ParameterParserFixture.cs

@@ -1,4 +1,5 @@
 using NHibernate.Engine.Query;
+using NSubstitute;
 using NUnit.Framework;
 
 namespace NHibernate.Test.EngineTest
@@ -19,8 +20,8 @@ FROM tablea
 
 			var recognizer = new ParamLocationRecognizer();
 			ParameterParser.Parse(query, recognizer);
-			ParamLocationRecognizer.NamedParameterDescription p;
-			Assert.DoesNotThrow(() => p = recognizer.NamedParameterDescriptionMap["name"]);
+			Assert.That(recognizer.NamedParameterDescriptionMap, Has.Count.EqualTo(1));
+			Assert.That(recognizer.NamedParameterDescriptionMap, Contains.Key("name"));
 		}
 
 		[Test]
@@ -30,14 +31,14 @@ public void CanFindParameterAfterInlineComment()
 	@"
 SELECT id 
 FROM tablea 
--- Comment with ' number 1 
+-- Comment with ' :number 1 
 WHERE Name = :name 
 ORDER BY Name";
 
 			var recognizer = new ParamLocationRecognizer();
 			ParameterParser.Parse(query, recognizer);
-			ParamLocationRecognizer.NamedParameterDescription p;
-			Assert.DoesNotThrow(() => p = recognizer.NamedParameterDescriptionMap["name"]);
+			Assert.That(recognizer.NamedParameterDescriptionMap, Has.Count.EqualTo(1));
+			Assert.That(recognizer.NamedParameterDescriptionMap, Contains.Key("name"));
 		}
 
 		[Test]
@@ -54,9 +55,41 @@ FROM tablea
 
 			var recognizer = new ParamLocationRecognizer();
 			ParameterParser.Parse(query, recognizer);
-			ParamLocationRecognizer.NamedParameterDescription p;
-			Assert.DoesNotThrow(() => p = recognizer.NamedParameterDescriptionMap["name"]);
-			Assert.DoesNotThrow(() => p = recognizer.NamedParameterDescriptionMap["pizza"]);
+			Assert.That(recognizer.NamedParameterDescriptionMap, Has.Count.EqualTo(2));
+			Assert.That(recognizer.NamedParameterDescriptionMap, Contains.Key("name"));
+			Assert.That(recognizer.NamedParameterDescriptionMap, Contains.Key("pizza"));
+		}
+
+		[Test]
+		public void IgnoresCommentsWithinQuotes()
+		{
+			string query =
+	@"
+SELECT id 
+FROM tablea WHERE Name = '
+-- :comment1
+' OR Name = '
+/* :comment2 */
+'
+-- :comment3
+/* :comment4 */
+ORDER BY Name + :pizza -- :comment5";
+
+			var recognizer = Substitute.For<ParameterParser.IRecognizer>();
+			ParameterParser.Parse(query, recognizer);
+
+			//Only one parameter in the query
+			recognizer.ReceivedWithAnyArgs(1).NamedParameter(default, default);
+			recognizer.Received(1).NamedParameter("pizza", Arg.Any<int>());
+
+			//comment1 and comment2 are not really comments and therefore not parsed as blocks
+			recognizer.DidNotReceive().Other(Arg.Is<string>(x => x.Contains("comment1")));
+			recognizer.DidNotReceive().Other(Arg.Is<string>(x => x.Contains("comment2")));
+
+			//comment 3-5 are actual comments and therefore parsed as blocks
+			recognizer.Received(1).Other(Arg.Is<string>(x => x.StartsWith("-- :comment3")));
+			recognizer.Received(1).Other("/* :comment4 */");
+			recognizer.Received(1).Other(Arg.Is<string>(x => x.StartsWith("-- :comment5")));
 		}
 	}
 }

src/NHibernate/Engine/Query/ParameterParser.cs

@@ -48,47 +48,47 @@ public static void Parse(string sqlString, IRecognizer recognizer)
 
 			int stringLength = sqlString.Length;
 			bool inQuote = false;
-			bool afterNewLine = false;
 			for (int indx = 0; indx < stringLength; indx++)
 			{
 				int currentNewLineLength;
 
-				// check comments
-				if (indx + 1 < stringLength && sqlString.Substring(indx,2) == "/*")
+				// check comments, unless in quote
+				if (!inQuote)
 				{
-					var closeCommentIdx = sqlString.IndexOf("*/", indx + 2, StringComparison.Ordinal);
-					recognizer.Other(sqlString.Substring(indx, (closeCommentIdx- indx)+2));
-					indx = closeCommentIdx + 1;
-					continue;
-				}
-				
-				if (afterNewLine && (indx + 1 < stringLength) && sqlString.Substring(indx, 2) == "--")
-				{
-					var closeCommentIdx = sqlString.IndexOfAnyNewLine(indx + 2, out currentNewLineLength);
-						
-					string comment;
-					if (closeCommentIdx == -1)
+					if (indx + 1 < stringLength && sqlString.Substring(indx, 2) == "/*")
 					{
-						closeCommentIdx = sqlString.Length;
-						comment = sqlString.Substring(indx);
+						var closeCommentIdx = sqlString.IndexOf("*/", indx + 2, StringComparison.Ordinal);
+						recognizer.Other(sqlString.Substring(indx, (closeCommentIdx - indx) + 2));
+						indx = closeCommentIdx + 1;
+						continue;
 					}
-					else
+
+					if ((indx + 1 < stringLength) && sqlString.Substring(indx, 2) == "--")
 					{
-						comment = sqlString.Substring(indx, closeCommentIdx - indx + currentNewLineLength);
+						var closeCommentIdx = sqlString.IndexOfAnyNewLine(indx + 2, out currentNewLineLength);
+
+						string comment;
+						if (closeCommentIdx == -1)
+						{
+							closeCommentIdx = sqlString.Length;
+							comment = sqlString.Substring(indx);
+						}
+						else
+						{
+							comment = sqlString.Substring(indx, closeCommentIdx - indx + currentNewLineLength);
+						}
+						recognizer.Other(comment);
+						indx = closeCommentIdx + currentNewLineLength - 1;
+						continue;
 					}
-					recognizer.Other(comment);
-					indx = closeCommentIdx + currentNewLineLength - 1;
-					continue;
 				}
 
 				if (sqlString.IsAnyNewLine(indx, out currentNewLineLength))
 				{
-					afterNewLine = true;
 					indx += currentNewLineLength - 1;
 					recognizer.Other(Environment.NewLine);
 					continue;
 				}
-				afterNewLine = false;
 
 				char c = sqlString[indx];
 				if (inQuote)