diff --git a/src/Microsoft.SqlTools.ManagedBatchParser/BatchParser/ExecutionEngineCode/ExecutionEngine.cs b/src/Microsoft.SqlTools.ManagedBatchParser/BatchParser/ExecutionEngineCode/ExecutionEngine.cs index 84c70dc8..935acbf4 100644 --- a/src/Microsoft.SqlTools.ManagedBatchParser/BatchParser/ExecutionEngineCode/ExecutionEngine.cs +++ b/src/Microsoft.SqlTools.ManagedBatchParser/BatchParser/ExecutionEngineCode/ExecutionEngine.cs @@ -19,7 +19,7 @@ using Microsoft.SqlTools.BatchParser.Utility; namespace Microsoft.SqlTools.ServiceLayer.BatchParser.ExecutionEngineCode { /// - /// Execution engine class which executed the parsed batches + /// Execution engine class which executes the parsed batches /// public class ExecutionEngine : IDisposable { diff --git a/src/Microsoft.SqlTools.ManagedBatchParser/BatchParser/Parser.cs b/src/Microsoft.SqlTools.ManagedBatchParser/BatchParser/Parser.cs index 18b11e12..d4d8d754 100644 --- a/src/Microsoft.SqlTools.ManagedBatchParser/BatchParser/Parser.cs +++ b/src/Microsoft.SqlTools.ManagedBatchParser/BatchParser/Parser.cs @@ -34,8 +34,18 @@ namespace Microsoft.SqlTools.ServiceLayer.BatchParser this.variableResolver = variableResolver; lexer = new Lexer(reader, name); tokenBuffer = new List(); + DisableVariableSubstitution = variableResolver == null; } + /// + /// Whether to enable or disable the variable substitution. + /// + public bool DisableVariableSubstitution { get; set; } + + /// + /// Whether to throw an error when a variable to be substituted was not specified. + /// + /// When DisableVariableSubstitution is true, this value is ignored, i.e. no exception will be thrown. public bool ThrowOnUnresolvedVariable { get; set; } private Token LookaheadToken @@ -110,9 +120,9 @@ namespace Microsoft.SqlTools.ServiceLayer.BatchParser private void AddVariableReferences(Token token, int offset, IList variableRefs) { - if (lexer.RecognizeSqlCmdSyntax == false) + if (!lexer.RecognizeSqlCmdSyntax || DisableVariableSubstitution) { - // variables are recognized only in sqlcmd mode. + // variables are recognized only in sqlcmd mode and the substitution was not explicitly disabled return; } @@ -649,11 +659,15 @@ namespace Microsoft.SqlTools.ServiceLayer.BatchParser column: column, offset: reference.Start - offset, filename: inputToken.Filename); - string value = variableResolver.GetVariable(variablePos.Value, reference.VariableName); + // If the variableResolver is not defined (=null), then we are a little + // more robust and avoid throwing a NRE. It may just mean that the caller + // did not bother implementing the IVariableResolver interface, presumably + // because they were not interested in any variable replacement. + string value = variableResolver?.GetVariable(variablePos.Value, reference.VariableName); if (value == null) { // Undefined variable - if (ThrowOnUnresolvedVariable == true) + if (ThrowOnUnresolvedVariable) { RaiseError(ErrorCode.VariableNotDefined, inputToken, string.Format(CultureInfo.CurrentCulture, SR.BatchParser_VariableNotDefined, reference.VariableName)); diff --git a/test/Microsoft.SqlTools.ManagedBatchParser.IntegrationTests/BatchParser/BatchParserTests.cs b/test/Microsoft.SqlTools.ManagedBatchParser.IntegrationTests/BatchParser/BatchParserTests.cs index 9a786b19..595b78c2 100644 --- a/test/Microsoft.SqlTools.ManagedBatchParser.IntegrationTests/BatchParser/BatchParserTests.cs +++ b/test/Microsoft.SqlTools.ManagedBatchParser.IntegrationTests/BatchParser/BatchParserTests.cs @@ -38,7 +38,184 @@ namespace Microsoft.SqlTools.ManagedBatchParser.UnitTests.BatchParser } /// - /// Verified that the parser throws an exception when the the sqlcmd script + /// Verifies that no exception is thrown when IVariableResolver passed to + /// the Parser ctor is null. + /// + [Test] + public void CanHandleNullIVariableResolver() + { + string script = @" +SELECT '$(VAR1)' +GO 10 +SELECT '$(VAR2)'"; + + StringBuilder output = new StringBuilder(); + + TestCommandHandler handler = new TestCommandHandler(output); + + using (var p = new Parser( + commandHandler: handler, + variableResolver: null, + new StringReader(script), + "test")) + { + p.ThrowOnUnresolvedVariable = false; + handler.SetParser(p); + + p.Parse(); + Assert.That(output.ToString(), Is.EqualTo("*** Execute batch (10)\nBatch text:\r\n\r\nSELECT '$(VAR1)'\r\n\r\n\r\n*** Execute batch (1)\nBatch text:\r\nSELECT '$(VAR2)'\r\n\r\n"), "Oh no!"); + } + } + + /// + /// Verifies that the default value for DisableVariableSubstitution on the + /// Parser object is false, unless IVariableResolver is null. + /// Essentially, this means that IVariableResolver=null implies + /// DisableVariableSubstitution=true. + /// + [TestCase(true, Description = "IVariableResolver is null", TestName = "DisableVariableSubstitutionIsTrueWhenIVariableResolverIsNull")] + [TestCase(false, Description = "IVariableResolver is not null", TestName = "DisableVariableSubstitutionIsFalseWhenIVariableResolverIsNotNull")] + public void DisableVariableSubstitutionTests(bool p) + { + string script = "SELECT $(VAR1)"; + + StringBuilder output = new StringBuilder(); + + TestCommandHandler handler = new TestCommandHandler(output); + + using (var parser = new Parser(commandHandler: handler, variableResolver: p ? null : new TestVariableResolver(output), new StringReader(script), "test")) + { + Assert.That(parser.DisableVariableSubstitution, Is.EqualTo(p), "Unexpected default value for DisableVariableSubstitution"); + } + } + + /// + /// Shows how the DisableVariableSubstitution, ThrowOnUnresolvedVariable, and the success + /// of failure of a substitution interact with one another. + /// + [TestCase(true, true, true)] + [TestCase(true, true, false)] + [TestCase(true, false, true)] + [TestCase(true, false, false)] + [TestCase(false, true, true)] + [TestCase(false, true, false)] + [TestCase(false, false, true)] + [TestCase(false, false, false)] + public void DisableVariableSubstitutionAndThrowOnUnresolvedVariableInteraction(bool disableVariableSubstitution, bool throwOnUnresolvedVariable, bool canResolve) + { + string script = "SELECT $(VAR1)"; + var output_hander = new StringBuilder(); + var output_resolver = new StringBuilder(); + var handler = new TestCommandHandler(output_hander); + var resolver = new TestVariableResolver(output_resolver); + + if (canResolve) + { + resolver.SetVariable(new PositionStruct(), "VAR1", "42"); + } + + using (var parser = new Parser(commandHandler: handler, variableResolver: resolver, new StringReader(script), "test")) + { + parser.DisableVariableSubstitution = disableVariableSubstitution; + parser.ThrowOnUnresolvedVariable = throwOnUnresolvedVariable; + + if (disableVariableSubstitution || canResolve || !throwOnUnresolvedVariable) + { + parser.Parse(); + if (canResolve && !disableVariableSubstitution) + { + // We do not really care about the whole output... a partial match is sufficient. + Assert.That(output_hander.ToString(), Contains.Substring("Execute batch (1)\nText with variables resolved:\r\nSELECT 42\r\nText with variables not resolved:\r\nSELECT $(VAR1)"), "Unexpected result of parsing!"); + } + else + { + // We do not really care about the whole output... a partial match is sufficient. + Assert.That(output_hander.ToString(), Is.EqualTo("*** Execute batch (1)\nBatch text:\r\nSELECT $(VAR1)\r\n\r\n"), "Unexpected result of parsing!"); + } + } + else + { + var exc = Assert.Throws(parser.Parse, "Expected exception because $(VAR1) was not defined!"); + Assert.That(exc.ErrorCode, Is.EqualTo(Microsoft.SqlTools.ServiceLayer.BatchParser.ErrorCode.VariableNotDefined), "Error code should be VariableNotDefined!"); + Assert.That(exc.TokenType, Is.EqualTo(LexerTokenType.Text), "Unexpected TokenType"); + Assert.That(exc.Text, Is.EqualTo("SELECT $(VAR1)"), "Unexpected Text"); + } + } + } + + + [Test] + [Ignore("Active issue: https://github.com/microsoft/sqltoolsservice/issues/1938")] + public void BatchParserCanHandleSqlAgentTokens() + { + string script = "SELECT N'$(ESCAPE_SQUOTE(SRVR))'"; + + StringBuilder output = new StringBuilder(); + + TestCommandHandler handler = new TestCommandHandler(output); + + using (var parser = new Parser(commandHandler: handler, new TestVariableResolver(output), new StringReader(script), "test")) + { + var exc = Assert.Throws(parser.Parse, "Was https://github.com/microsoft/sqltoolsservice/issues/1938 fixed? Please, update the test!"); + Assert.That(exc.ErrorCode, Is.EqualTo(Microsoft.SqlTools.ServiceLayer.BatchParser.ErrorCode.InvalidVariableName), "Error code should be InvalidVariableName!"); + Assert.That(exc.TokenType, Is.EqualTo(LexerTokenType.Text), "Unexpected TokenType"); + Assert.That(exc.Text, Is.EqualTo("$(ESCAPE_SQUOTE("), "Unexpected Text"); + } + } + + /// + /// Setting DisableVariableSubstitution=true has the effect of preventing the + /// Parser from trying to interpret variables, thus allowing such variables to + /// remain embedded in strings within the T-SQL script (e.g. SQL Agent variables) + /// + /// This test will need to be modified when issue 1938 is addressed. + [Test] + public void WhenDisableVariableSubstitutionIsTrueNoParsingOfVariablesHappens() + { + string script = "SELECT N'$(ESCAPE_SQUOTE(SRVR))'"; + + StringBuilder output = new StringBuilder(); + + TestCommandHandler handler = new TestCommandHandler(output); + + using (var parser = new Parser(commandHandler: handler, new TestVariableResolver(output), new StringReader(script), "test")) + { + // Explicitly disable variable substitution + parser.DisableVariableSubstitution = true; + + parser.Parse(); + Assert.That(output.ToString(), Is.EqualTo("*** Execute batch (1)\nBatch text:\r\nSELECT N'$(ESCAPE_SQUOTE(SRVR))'\r\n\r\n"), "How was the SQL Agent macro parsed?"); + } + } + + /// + /// Setting DisableVariableSubstitution=true has the effect of preventing the + /// Parser from trying to interpret variables; without this, we would not be + /// able to handle T-SQL fragement (like strings) that happen to have in them + /// text that resemble a sqlcmd variable, e.g. "$(". + /// + /// This test will need to be modified when issue 1938 is addressed. + [Test] + public void WhenDisableVariableSubstitutionIsTrueNoParsingOfVariablesHappensWithMalformedVariable() + { + string script = @"SELECT N'$(X'"; // Note: $(X is a valid string in T-SQL (it is enclosed in single quotes, but it has nothing to do with a variable!) + + StringBuilder output = new StringBuilder(); + + TestCommandHandler handler = new TestCommandHandler(output); + + using (var parser = new Parser(commandHandler: handler, new TestVariableResolver(output), new StringReader(script), "test")) + { + // Explicitly disable variable substitution + parser.DisableVariableSubstitution = true; + + parser.Parse(); + Assert.That(output.ToString(), Is.EqualTo("*** Execute batch (1)\nBatch text:\r\nSELECT N'$(X'\r\n\r\n"), "Why are we trying to make sense of $(X ?"); + } + } + + /// + /// Verifies that the parser throws an exception when the the sqlcmd script /// uses a variable that is not defined. The expected exception has the /// correct ErrorCode and TokenType. /// @@ -61,7 +238,7 @@ namespace Microsoft.SqlTools.ManagedBatchParser.UnitTests.BatchParser var exc = Assert.Throws(p.Parse, "Expected exception because $(NotDefined) was not defined!"); Assert.That(exc.ErrorCode, Is.EqualTo(Microsoft.SqlTools.ServiceLayer.BatchParser.ErrorCode.VariableNotDefined), "Error code should be VariableNotDefined!"); - Assert.That(exc.TokenType, Is.EqualTo(LexerTokenType.Text), "Token"); + Assert.That(exc.TokenType, Is.EqualTo(LexerTokenType.Text), "Unexpected TokenType"); } } @@ -83,8 +260,8 @@ namespace Microsoft.SqlTools.ManagedBatchParser.UnitTests.BatchParser handler.SetParser(p); var exc = Assert.Throws(p.Parse, "Expected exception because $(0alcOne) was not defined!"); Assert.That(exc.ErrorCode, Is.EqualTo(Microsoft.SqlTools.ServiceLayer.BatchParser.ErrorCode.InvalidVariableName), "Error code should be InvalidVariableName!"); - Assert.That(exc.TokenType, Is.EqualTo(LexerTokenType.Text), "Token"); - Assert.That(exc.Text, Is.EqualTo("$(0"), "Token"); + Assert.That(exc.TokenType, Is.EqualTo(LexerTokenType.Text), "Unexpected TokenType"); + Assert.That(exc.Text, Is.EqualTo("$(0"), "Unexpected Text"); } } @@ -107,8 +284,8 @@ namespace Microsoft.SqlTools.ManagedBatchParser.UnitTests.BatchParser var exc = Assert.Throws(p.Parse, "Expected exception because $(ca@lcOne) was not defined!"); Assert.That(exc.ErrorCode, Is.EqualTo(Microsoft.SqlTools.ServiceLayer.BatchParser.ErrorCode.InvalidVariableName), "Error code should be InvalidVariableName!"); - Assert.That(exc.TokenType, Is.EqualTo(LexerTokenType.Text), "Token"); - Assert.That(exc.Text, Is.EqualTo("$(ca@"), "Token"); + Assert.That(exc.TokenType, Is.EqualTo(LexerTokenType.Text), "Unexpected TokenType"); + Assert.That(exc.Text, Is.EqualTo("$(ca@"), "Unexpected Text"); } } @@ -134,8 +311,8 @@ namespace Microsoft.SqlTools.ManagedBatchParser.UnitTests.BatchParser // Exception will be raised from ParseGo() var exc = Assert.Throws(p.Parse, $"Expected exception because GO is followed by a invalid number (>{int.MaxValue})"); Assert.That(exc.ErrorCode, Is.EqualTo(Microsoft.SqlTools.ServiceLayer.BatchParser.ErrorCode.InvalidNumber), "Error code should be InvalidNumber!"); - Assert.That(exc.TokenType, Is.EqualTo(LexerTokenType.Text), "Token"); - Assert.That(exc.Text, Is.EqualTo("2147483648"), "Token"); + Assert.That(exc.TokenType, Is.EqualTo(LexerTokenType.Text), "Unexpected TokenType"); + Assert.That(exc.Text, Is.EqualTo("2147483648"), "Unexpected Text"); } } diff --git a/test/Microsoft.SqlTools.ManagedBatchParser.IntegrationTests/BatchParser/TestCommandHandler.cs b/test/Microsoft.SqlTools.ManagedBatchParser.IntegrationTests/BatchParser/TestCommandHandler.cs index 49283834..aa8dc5c6 100644 --- a/test/Microsoft.SqlTools.ManagedBatchParser.IntegrationTests/BatchParser/TestCommandHandler.cs +++ b/test/Microsoft.SqlTools.ManagedBatchParser.IntegrationTests/BatchParser/TestCommandHandler.cs @@ -41,7 +41,7 @@ namespace Microsoft.SqlTools.ManagedBatchParser.UnitTests.BatchParser if (string.Compare(textWithVariablesUnresolved, textWithVariablesResolved, StringComparison.Ordinal) != 0) { - outputString.AppendLine("Text with variables not resolved:"); + outputString.AppendLine("Text with variables resolved:"); outputString.AppendLine(textWithVariablesResolved); outputString.AppendLine("Text with variables not resolved:"); outputString.AppendLine(textWithVariablesUnresolved);