Skip to content

Commit

Permalink
Fix NullRefEx bug when accessing scriptFile.ReferencedFiles (#838)
Browse files Browse the repository at this point in the history
* 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.  :-(

* Revise ScriptFile prop init to happen within ParseFileContents & add tests
  • Loading branch information
rkeithhill authored and TylerLeonhardt committed Jan 8, 2019
1 parent 3505902 commit 73044d1
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 10 deletions.
19 changes: 13 additions & 6 deletions src/PowerShellEditorServices/Workspace/ScriptFile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ public class ScriptFile
"\n"
};

private Token[] scriptTokens;
private Version powerShellVersion;

#endregion
Expand Down Expand Up @@ -116,7 +115,8 @@ public ScriptBlockAst ScriptAst
/// </summary>
public Token[] ScriptTokens
{
get { return this.scriptTokens; }
get;
private set;
}

/// <summary>
Expand Down Expand Up @@ -152,6 +152,7 @@ public ScriptFile(
this.IsInMemory = Workspace.IsPathInMemory(filePath);
this.powerShellVersion = powerShellVersion;

// SetFileContents() calls ParseFileContents() which initializes the rest of the properties.
this.SetFileContents(textReader.ReadToEnd());
}

Expand Down Expand Up @@ -599,6 +600,8 @@ private void ParseFileContents()

try
{
Token[] scriptTokens;

#if PowerShellv5r2
// This overload appeared with Windows 10 Update 1
if (this.powerShellVersion.Major >= 5 &&
Expand All @@ -610,24 +613,26 @@ private void ParseFileContents()
Parser.ParseInput(
this.Contents,
this.FilePath,
out this.scriptTokens,
out scriptTokens,
out parseErrors);
}
else
{
this.ScriptAst =
Parser.ParseInput(
this.Contents,
out this.scriptTokens,
out scriptTokens,
out parseErrors);
}
#else
this.ScriptAst =
Parser.ParseInput(
this.Contents,
out this.scriptTokens,
out scriptTokens,
out parseErrors);
#endif

this.ScriptTokens = scriptTokens;
}
catch (RuntimeException ex)
{
Expand All @@ -638,7 +643,7 @@ private void ParseFileContents()
ex.Message);

parseErrors = new[] { parseError };
this.scriptTokens = new Token[0];
this.ScriptTokens = new Token[0];
this.ScriptAst = null;
}

Expand All @@ -654,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;
}

Expand Down
55 changes: 51 additions & 4 deletions test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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);
}
}
}
}

0 comments on commit 73044d1

Please sign in to comment.