From bd01d798725bdf1b24dea87a7e4c57d323bd39b2 Mon Sep 17 00:00:00 2001 From: Keith Hill Date: Fri, 28 Dec 2018 12:39:36 -0700 Subject: [PATCH 1/2] Fix NullRefEx bug when accessing scriptFile.ReferencedFilesThis happens because the ScriptFile ctor does not initialize all itspublic props. I added initialization for the other public propsexcept for the ScriptAst prop. I don't see an empty Ast. Perhaps nullis OK for this prop?This address vscode-powershell bug https://github.com/PowerShell/vscode-powershell/issues/1675Also, for the 2.0.0 branch, we should see if we can use Array.Empty<>()for initialization. It isn't availble to net45x. :-( --- .../Workspace/ScriptFile.cs | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/PowerShellEditorServices/Workspace/ScriptFile.cs b/src/PowerShellEditorServices/Workspace/ScriptFile.cs index dab2918cc..21ec14f20 100644 --- a/src/PowerShellEditorServices/Workspace/ScriptFile.cs +++ b/src/PowerShellEditorServices/Workspace/ScriptFile.cs @@ -26,7 +26,6 @@ public class ScriptFile "\n" }; - private Token[] scriptTokens; private Version powerShellVersion; #endregion @@ -116,7 +115,8 @@ public ScriptBlockAst ScriptAst /// public Token[] ScriptTokens { - get { return this.scriptTokens; } + get; + private set; } /// @@ -146,11 +146,16 @@ public ScriptFile( TextReader textReader, Version powerShellVersion) { + this.powerShellVersion = powerShellVersion; + this.FilePath = filePath; this.ClientFilePath = clientFilePath; this.IsAnalysisEnabled = true; this.IsInMemory = Workspace.IsPathInMemory(filePath); - this.powerShellVersion = powerShellVersion; + this.ReferencedFiles = new string[0]; + this.SyntaxMarkers = new ScriptFileMarker[0]; + this.FileLines = new List(); + this.ScriptTokens = new Token[0]; this.SetFileContents(textReader.ReadToEnd()); } @@ -599,6 +604,8 @@ private void ParseFileContents() try { + Token[] scriptTokens; + #if PowerShellv5r2 // This overload appeared with Windows 10 Update 1 if (this.powerShellVersion.Major >= 5 && @@ -610,7 +617,7 @@ private void ParseFileContents() Parser.ParseInput( this.Contents, this.FilePath, - out this.scriptTokens, + out scriptTokens, out parseErrors); } else @@ -618,14 +625,16 @@ private void ParseFileContents() this.ScriptAst = Parser.ParseInput( this.Contents, - out this.scriptTokens, + out scriptTokens, out parseErrors); } + + this.ScriptTokens = scriptTokens; #else this.ScriptAst = Parser.ParseInput( this.Contents, - out this.scriptTokens, + out scriptTokens, out parseErrors); #endif } @@ -638,7 +647,7 @@ private void ParseFileContents() ex.Message); parseErrors = new[] { parseError }; - this.scriptTokens = new Token[0]; + this.ScriptTokens = new Token[0]; this.ScriptAst = null; } From d3644a2bfa7e31beac36d68cdd8f653a48414f08 Mon Sep 17 00:00:00 2001 From: Keith Hill Date: Wed, 2 Jan 2019 21:59:49 -0700 Subject: [PATCH 2/2] Revise ScriptFile prop init to happen within ParseFileContents & add tests --- .../Workspace/ScriptFile.cs | 14 ++--- .../Session/ScriptFileTests.cs | 55 +++++++++++++++++-- 2 files changed, 57 insertions(+), 12 deletions(-) diff --git a/src/PowerShellEditorServices/Workspace/ScriptFile.cs b/src/PowerShellEditorServices/Workspace/ScriptFile.cs index 21ec14f20..0e0d4d6e7 100644 --- a/src/PowerShellEditorServices/Workspace/ScriptFile.cs +++ b/src/PowerShellEditorServices/Workspace/ScriptFile.cs @@ -146,17 +146,13 @@ public ScriptFile( TextReader textReader, Version powerShellVersion) { - this.powerShellVersion = powerShellVersion; - this.FilePath = filePath; this.ClientFilePath = clientFilePath; this.IsAnalysisEnabled = true; this.IsInMemory = Workspace.IsPathInMemory(filePath); - this.ReferencedFiles = new string[0]; - this.SyntaxMarkers = new ScriptFileMarker[0]; - this.FileLines = new List(); - this.ScriptTokens = new Token[0]; + this.powerShellVersion = powerShellVersion; + // SetFileContents() calls ParseFileContents() which initializes the rest of the properties. this.SetFileContents(textReader.ReadToEnd()); } @@ -628,8 +624,6 @@ private void ParseFileContents() out scriptTokens, out parseErrors); } - - this.ScriptTokens = scriptTokens; #else this.ScriptAst = Parser.ParseInput( @@ -637,6 +631,8 @@ private void ParseFileContents() out scriptTokens, out parseErrors); #endif + + this.ScriptTokens = scriptTokens; } catch (RuntimeException ex) { @@ -663,6 +659,8 @@ private void ParseFileContents() // users should save the file. if (IsUntitledPath(this.FilePath)) { + // Need to initialize the ReferencedFiles property to an empty array. + this.ReferencedFiles = new string[0]; return; } diff --git a/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs b/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs index adfc2727e..993e56999 100644 --- a/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs +++ b/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs @@ -164,10 +164,10 @@ public void CanAppendToEndOfFile() public void FindsDotSourcedFiles() { string exampleScriptContents = - @". .\athing.ps1"+"\r\n"+ - @". .\somefile.ps1"+"\r\n" + - @". .\somefile.ps1"+"\r\n" + - @"Do-Stuff $uri"+"\r\n" + + @". .\athing.ps1" + "\r\n" + + @". .\somefile.ps1" + "\r\n" + + @". .\somefile.ps1" + "\r\n" + + @"Do-Stuff $uri" + "\r\n" + @". simpleps.ps1"; using (StringReader stringReader = new StringReader(exampleScriptContents)) @@ -532,4 +532,51 @@ private void AssertNewPosition( Assert.Equal(expectedColumn, newPosition.Column); } } + + public class ScriptFileConstructorTests + { + private static readonly Version PowerShellVersion = new Version("5.0"); + + [Fact] + public void PropertiesInitializedCorrectlyForFile() + { + var path = "TestFile.ps1"; + var scriptFile = ScriptFileChangeTests.CreateScriptFile(""); + + Assert.Equal(path, scriptFile.FilePath); + Assert.Equal(path, scriptFile.ClientFilePath); + Assert.True(scriptFile.IsAnalysisEnabled); + Assert.False(scriptFile.IsInMemory); + Assert.Empty(scriptFile.ReferencedFiles); + Assert.Empty(scriptFile.SyntaxMarkers); + Assert.Single(scriptFile.ScriptTokens); + Assert.Single(scriptFile.FileLines); + } + + [Fact] + public void PropertiesInitializedCorrectlyForUntitled() + { + var path = "untitled:untitled-1"; + + // 3 lines and 10 tokens in this script. + var script = @"function foo() { + 'foo' +}"; + + using (StringReader stringReader = new StringReader(script)) + { + // Create an in-memory file from the StringReader + var scriptFile = new ScriptFile(path, path, stringReader, PowerShellVersion); + + Assert.Equal(path, scriptFile.FilePath); + Assert.Equal(path, scriptFile.ClientFilePath); + Assert.True(scriptFile.IsAnalysisEnabled); + Assert.True(scriptFile.IsInMemory); + Assert.Empty(scriptFile.ReferencedFiles); + Assert.Empty(scriptFile.SyntaxMarkers); + Assert.Equal(10, scriptFile.ScriptTokens.Length); + Assert.Equal(3, scriptFile.FileLines.Count); + } + } + } }