(cake-buildGH-1746) ScriptAnalyzer will not throw on error.

- ScriptAnalyzer will not throw on error. Instead the result will
indicate if analysis succeeded or not. A list of errors is also present
in the analyzer result.
- Fixes cake-build#1746

src/Cake.Core.Tests/Unit/Scripting/Analysis/ScriptAnalyzerResultTests.cs

@@ -107,6 +107,33 @@ public void Should_Populate_Tools_From_Provided_Script()
                 Assert.Contains(result.Tools, package => package.OriginalString == "nuget:?package=First.Package");
                 Assert.Contains(result.Tools, package => package.OriginalString == "nuget:?package=Second.Package");
             }
+
+            [Fact]
+            public void Should_Set_Succeeded_To_False_If_Any_Errors()
+            {
+                // Given
+                var script = new ScriptInformation("./build.cake");
+                var error = new ScriptAnalyzerError("./build.cake", 1, "error message");
+
+                // When
+                var result = new ScriptAnalyzerResult(script, new List<string>(), new List<ScriptAnalyzerError> { error });
+
+                // Then
+                Assert.False(result.Succeeded);
+            }
+
+            [Fact]
+            public void Should_Set_Succeeded_To_True_If_Not_Any_Errors()
+            {
+                // Given
+                var script = new ScriptInformation("./build.cake");
+
+                // When
+                var result = new ScriptAnalyzerResult(script, new List<string>(), new List<ScriptAnalyzerError>());
+
+                // Then
+                Assert.True(result.Succeeded);
+            }
         }
     }
 }

src/Cake.Core.Tests/Unit/Scripting/Analysis/ScriptAnalyzerTests.cs

@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 using System.Linq;
+using Cake.Core.Scripting.Processors.Loading;
 using Cake.Core.Tests.Fixtures;
 using Xunit;
 
@@ -71,17 +72,18 @@ public void Should_Throw_If_File_Path_Is_Null()
             }
 
             [Fact]
-            public void Should_Throw_If_Script_Was_Not_Found()
+            public void Should_Return_Error_If_Script_Was_Not_Found()
             {
                 // Given
                 var fixture = new ScriptAnalyzerFixture();
 
                 // When
-                var result = Record.Exception(() => fixture.Analyze("/Working/notfound.cake"));
+                var result = fixture.Analyze("/Working/notfound.cake");
 
                 // Then
-                Assert.IsType<CakeException>(result);
-                Assert.Equal("Could not find script '/Working/notfound.cake'.", result?.Message);
+                Assert.False(result.Succeeded);
+                Assert.Equal(1, result.Errors.Count);
+                Assert.Equal("Could not find script '/Working/notfound.cake'.", result.Errors[0].Message);
             }
 
             [Fact]
@@ -324,6 +326,49 @@ public void Should_Process_Break_Directive()
                 Assert.Equal(result.Lines[0], "#line 1 \"/Working/script.cake\"");
                 Assert.Equal(result.Lines[1], @"if (System.Diagnostics.Debugger.IsAttached) { System.Diagnostics.Debugger.Break(); }");
             }
+
+            [Fact]
+            public void Should_Log_Error_Location()
+            {
+                // Given
+                var fixture = new ScriptAnalyzerFixture();
+                fixture.Providers.Add(new FileLoadDirectiveProvider());
+                fixture.GivenScriptExist("/Working/script.cake", "#load \"local:?pat\"");
+
+                // When
+                var result = fixture.Analyze("/Working/script.cake");
+
+                // Then
+                Assert.Equal(2, result.Lines.Count);
+                Assert.Equal("#line 1 \"/Working/script.cake\"", result.Lines[0]);
+                Assert.Equal("// #load \"local:?pat\"", result.Lines[1]);
+                Assert.False(result.Succeeded);
+                Assert.Equal(1, result.Errors.Count);
+                Assert.Equal("/Working/script.cake", result.Errors[0].File.FullPath);
+                Assert.Equal(1, result.Errors[0].Line);
+                Assert.Equal("Query string for #load is missing parameter 'path'.", result.Errors[0].Message);
+            }
+
+            [Fact]
+            public void Should_Log_Error_Location_In_Loaded_Script()
+            {
+                // Given
+                var fixture = new ScriptAnalyzerFixture();
+                fixture.Providers.Add(new FileLoadDirectiveProvider());
+                fixture.GivenScriptExist("/Working/script.cake", "#load \"local:?path=script2.cake\"");
+                fixture.GivenScriptExist("/Working/script2.cake", "\n#load \"local:?path=1&path=2\"");
+
+                // When
+                var result = fixture.Analyze("/Working/script.cake");
+
+                // Then
+                Assert.Equal(6, result.Lines.Count);
+                Assert.False(result.Succeeded);
+                Assert.Equal(1, result.Errors.Count);
+                Assert.Equal("/Working/script2.cake", result.Errors[0].File.FullPath);
+                Assert.Equal(2, result.Errors[0].Line);
+                Assert.Equal("Query string for #load contains more than one parameter 'path'.", result.Errors[0].Message);
+            }
         }
     }
 }

src/Cake.Core/Scripting/Analysis/IScriptAnalyzerContext.cs

@@ -33,5 +33,11 @@ public interface IScriptAnalyzerContext
         /// </summary>
         /// <param name="line">The script line to add.</param>
         void AddScriptLine(string line);
+
+        /// <summary>
+        /// Adds a script error to the result.
+        /// </summary>
+        /// <param name="error">The script error to add.</param>
+        void AddScriptError(string error);
     }
 }

src/Cake.Core/Scripting/Analysis/ScriptAnalyzer.cs

@@ -92,7 +92,8 @@ public ScriptAnalyzerResult Analyze(FilePath path)
             // Create and return the results.
             return new ScriptAnalyzerResult(
                 context.Current,
-                context.Lines);
+                context.Lines,
+                context.Errors);
         }
 
         [SuppressMessage("ReSharper", "ConvertIfStatementToConditionalTernaryExpression")]

src/Cake.Core/Scripting/Analysis/ScriptAnalyzerContext.cs

@@ -20,6 +20,7 @@ internal sealed class ScriptAnalyzerContext : IScriptAnalyzerContext
         private readonly Stack<ScriptInformation> _stack;
         private readonly List<string> _lines;
         private readonly HashSet<FilePath> _processedScripts;
+        private readonly List<ScriptAnalyzerError> _errors;
         private ScriptInformation _current;
 
         // ReSharper disable once ConvertToAutoProperty
@@ -29,6 +30,8 @@ internal sealed class ScriptAnalyzerContext : IScriptAnalyzerContext
 
         public IReadOnlyList<string> Lines => _lines;
 
+        public IReadOnlyList<ScriptAnalyzerError> Errors => _errors;
+
         public ScriptAnalyzerContext(
             IFileSystem fileSystem,
             ICakeEnvironment environment,
@@ -44,6 +47,7 @@ public ScriptAnalyzerContext(
             _processedScripts = new HashSet<FilePath>(new PathComparer(_environment));
             _stack = new Stack<ScriptInformation>();
             _lines = new List<string>();
+            _errors = new List<ScriptAnalyzerError>();
         }
 
         public void Analyze(FilePath path)
@@ -60,7 +64,9 @@ public void Analyze(FilePath path)
             {
                 const string format = "Could not find script '{0}'.";
                 var message = string.Format(CultureInfo.InvariantCulture, format, path.FullPath);
-                throw new CakeException(message);
+                _errors.Add(new ScriptAnalyzerError(path, 0, message));
+                _current = new ScriptInformation(path);
+                return;
             }
 
             if (_processedScripts.Contains(path))
@@ -86,6 +92,14 @@ public void AddScriptLine(string line)
             _current.Lines.Add(line);
         }
 
+        public void AddScriptError(string error)
+        {
+            if (error != null)
+            {
+                _errors.Add(new ScriptAnalyzerError(_current.Path, _current.Lines.Count + 1, error));
+            }
+        }
+
         public void Push(FilePath path)
         {
             var script = new ScriptInformation(path);

src/Cake.Core/Scripting/Analysis/ScriptAnalyzerError.cs

@@ -0,0 +1,48 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System;
+using Cake.Core.IO;
+
+namespace Cake.Core.Scripting.Analysis
+{
+    /// <summary>
+    /// Represents a script analysis error.
+    /// </summary>
+    public sealed class ScriptAnalyzerError
+    {
+        /// <summary>
+        /// Gets the file containing the error.
+        /// </summary>
+        public FilePath File { get; }
+
+        /// <summary>
+        /// Gets the line number for the error.
+        /// </summary>
+        public int Line { get; }
+
+        /// <summary>
+        /// Gets the error message.
+        /// </summary>
+        public string Message { get; }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="ScriptAnalyzerError"/> class.
+        /// </summary>
+        /// <param name="file">The file containing the error.</param>
+        /// <param name="line">The line number for the error.</param>
+        /// <param name="message">The error message.</param>
+        public ScriptAnalyzerError(FilePath file, int line, string message)
+        {
+            if (file == null)
+            {
+                throw new ArgumentNullException(nameof(file));
+            }
+
+            File = file;
+            Line = line;
+            Message = message;
+        }
+    }
+}

src/Cake.Core/Scripting/Analysis/ScriptAnalyzerResult.cs

@@ -59,12 +59,23 @@ public sealed class ScriptAnalyzerResult
         /// <value>The tools.</value>
         public HashSet<PackageReference> Tools { get; }
 
+        /// <summary>
+        /// Gets a value indicating whether to analysis succeded without errors.
+        /// </summary>
+        public bool Succeeded { get; }
+
+        /// <summary>
+        /// Gets the list of analyzer errors.
+        /// </summary>
+        public IReadOnlyList<ScriptAnalyzerError> Errors { get; }
+
         /// <summary>
         /// Initializes a new instance of the <see cref="ScriptAnalyzerResult"/> class.
         /// </summary>
         /// <param name="script">The script.</param>
         /// <param name="lines">The merged script lines.</param>
-        public ScriptAnalyzerResult(IScriptInformation script, IReadOnlyList<string> lines)
+        /// <param name="errors">The analyzer errors.</param>
+        public ScriptAnalyzerResult(IScriptInformation script, IReadOnlyList<string> lines, IReadOnlyList<ScriptAnalyzerError> errors = null)
         {
             Script = script;
             Lines = lines;
@@ -73,9 +84,11 @@ public ScriptAnalyzerResult(IScriptInformation script, IReadOnlyList<string> lin
             _usingAliases = new HashSet<string>(Collect(script, i => i.UsingAliases));
             Tools = new HashSet<PackageReference>(Collect(script, i => i.Tools));
             Addins = new HashSet<PackageReference>(Collect(script, i => i.Addins));
+            Errors = errors ?? new List<ScriptAnalyzerError>(0);
+            Succeeded = Errors.Count == 0;
         }
 
-        private IEnumerable<T> Collect<T>(IScriptInformation script, Func<IScriptInformation, IEnumerable<T>> collector)
+        private static IEnumerable<T> Collect<T>(IScriptInformation script, Func<IScriptInformation, IEnumerable<T>> collector)
         {
             var stack = new Stack<IScriptInformation>();
             stack.Push(script);

src/Cake.Core/Scripting/Processors/UriDirectiveProcessor.cs

@@ -37,8 +37,16 @@ public sealed override bool Process(IScriptAnalyzerContext context, string line,
                         var uri = ParseUriFromTokens(tokens);
                         if (uri != null)
                         {
-                            // Add the URI to the context.
-                            AddToContext(context, uri);
+                            try
+                            {
+                                // Add the URI to the context.
+                                AddToContext(context, uri);
+                            }
+                            catch (Exception e)
+                            {
+                                // Add any errors to context
+                                context.AddScriptError(e.Message);
+                            }
 
                             // Return success.
                             return true;

src/Cake.Core/Scripting/ScriptRunner.cs

@@ -131,6 +131,17 @@ public void Run(IScriptHost host, FilePath scriptPath, IDictionary<string, strin
             _log.Verbose("Analyzing build script...");
             var result = _analyzer.Analyze(scriptPath.GetFilename());
 
+            // Log all errors and throw
+            if (!result.Succeeded)
+            {
+                foreach (var error in result.Errors)
+                {
+                    var format = $"{error.File.MakeAbsolute(_environment).FullPath}:{error.Line}: {{0}}";
+                    _log.Error(format, error.Message);
+                }
+                throw new CakeException("Errors occured while analyzing script.");
+            }
+
             // Install tools.
             _log.Verbose("Processing build script...");
             var toolsPath = GetToolPath(scriptPath.GetDirectory());