Simplify Workspace file operations and fix bugs

daviwil · Kayla Davis · commit 57392ba471e0 · 2015-08-10T15:55:21.000-07:00
diff --git a/src/PowerShellEditorServices.Transport.Stdio/Request/DeclarationRequest.cs b/src/PowerShellEditorServices.Transport.Stdio/Request/DeclarationRequest.cs
@@ -26,8 +26,9 @@ public override void ProcessMessage(
 
             GetDefinitionResult definition =
                 editorSession.LanguageService.GetDefinitionOfSymbol(
+                    scriptFile,
                     foundSymbol,
-                    editorSession.Workspace.ExpandScriptReferences(scriptFile));
+                    editorSession.Workspace);
 
             if (definition != null)
             {
diff --git a/src/PowerShellEditorServices.Transport.Stdio/Request/ErrorRequest.cs b/src/PowerShellEditorServices.Transport.Stdio/Request/ErrorRequest.cs
@@ -33,14 +33,9 @@ public override void ProcessMessage(
             // Get the requested files
             foreach (string filePath in this.Arguments.Files)
             {
-                ScriptFile scriptFile = null;
-
-                if (!editorSession.Workspace.TryGetFile(filePath, out scriptFile))
-                {
-                    // Skip this file and log the file load error
-                    // TODO: Trace out the error message
-                    continue;
-                }
+                ScriptFile scriptFile = 
+                    editorSession.Workspace.GetFile(
+                        filePath);
 
                 var semanticMarkers =
                     editorSession.AnalysisService.GetSemanticMarkers(
diff --git a/src/PowerShellEditorServices.Transport.Stdio/Request/FileRequest.cs b/src/PowerShellEditorServices.Transport.Stdio/Request/FileRequest.cs
@@ -13,20 +13,9 @@ public abstract class FileRequest<TArguments> : RequestBase<TArguments>
     {
         protected ScriptFile GetScriptFile(EditorSession editorSession)
         {
-            ScriptFile scriptFile = null;
-
-            if(!editorSession.Workspace.TryGetFile(
-                this.Arguments.File, 
-                out scriptFile))
-            {
-                // TODO: Throw an exception that the message loop can create a response out of
-
-                throw new FileNotFoundException(
-                    "A ScriptFile with the following path was not found in the EditorSession: {0}",
+            return 
+                editorSession.Workspace.GetFile(
                     this.Arguments.File);
-            }
-
-            return scriptFile;
         }
     }
 
diff --git a/src/PowerShellEditorServices.Transport.Stdio/Request/OpenFileRequest.cs b/src/PowerShellEditorServices.Transport.Stdio/Request/OpenFileRequest.cs
@@ -26,14 +26,8 @@ public override void ProcessMessage(
             EditorSession editorSession,
             MessageWriter messageWriter)
         {
-            ScriptFile scriptFile = null;
-
-            // Only load the file if it isn't loaded already
-            if (!editorSession.Workspace.TryGetFile(this.Arguments.File, out scriptFile))
-            {
-                // Open the file in the current session
-                editorSession.Workspace.OpenFile(this.Arguments.File);
-            }
+            // Open the file in the current session
+            editorSession.Workspace.GetFile(this.Arguments.File);
         }
     }
 }
diff --git a/src/PowerShellEditorServices/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Analysis/AnalysisService.cs
@@ -58,26 +58,34 @@ public AnalysisService(Runspace analysisRunspace)
         /// <returns>An array of ScriptFileMarkers containing semantic analysis results.</returns>
         public ScriptFileMarker[] GetSemanticMarkers(ScriptFile file)
         {
-            // TODO: This is a temporary fix until we can change how
-            // ScriptAnalyzer invokes their async tasks.
-            Task<ScriptFileMarker[]> analysisTask =
-                Task.Factory.StartNew<ScriptFileMarker[]>(
-                    () =>
-                    {
-                        return 
-                            this.scriptAnalyzer
-                                .AnalyzeSyntaxTree(
-                                    file.ScriptAst,
-                                    file.ScriptTokens,
-                                    file.FilePath)
-                                .Select(ScriptFileMarker.FromDiagnosticRecord)
-                                .ToArray();
-                    },
-                    CancellationToken.None,
-                    TaskCreationOptions.None,
-                    TaskScheduler.Default);
+            if (file.IsAnalysisEnabled)
+            {
+                // TODO: This is a temporary fix until we can change how
+                // ScriptAnalyzer invokes their async tasks.
+                Task<ScriptFileMarker[]> analysisTask =
+                    Task.Factory.StartNew<ScriptFileMarker[]>(
+                        () =>
+                        {
+                            return 
+                                this.scriptAnalyzer
+                                    .AnalyzeSyntaxTree(
+                                        file.ScriptAst,
+                                        file.ScriptTokens,
+                                        file.FilePath)
+                                    .Select(ScriptFileMarker.FromDiagnosticRecord)
+                                    .ToArray();
+                        },
+                        CancellationToken.None,
+                        TaskCreationOptions.None,
+                        TaskScheduler.Default);
 
-            return analysisTask.Result;
+                return analysisTask.Result;
+            }
+            else
+            {
+                // Return an empty marker list
+                return new ScriptFileMarker[0];
+            }
         }
 
         #endregion
diff --git a/src/PowerShellEditorServices/Language/LanguageService.cs b/src/PowerShellEditorServices/Language/LanguageService.cs
@@ -10,10 +10,8 @@
 namespace Microsoft.PowerShell.EditorServices.Language
 {
     using Microsoft.PowerShell.EditorServices.Utility;
-    using System.IO;
     using System.Management.Automation;
     using System.Management.Automation.Runspaces;
-    using System.Text;
 
     /// <summary>
     /// Provides a high-level service for performing code completion and
@@ -185,6 +183,7 @@ public FindReferencesResult FindReferencesOfSymbol(
                                 reference.FilePath = file.FilePath;
                                 return reference;
                             });
+
                     symbolReferences.AddRange(symbolReferencesinFile);
                 }
 
@@ -206,10 +205,17 @@ public FindReferencesResult FindReferencesOfSymbol(
         /// <param name="referencedFiles">An array of scriptFiles too search for the definition in</param>
         /// <returns>GetDefinitionResult</returns>
         public GetDefinitionResult GetDefinitionOfSymbol(
+            ScriptFile sourceFile,
             SymbolReference foundSymbol,
-            ScriptFile[] referencedFiles)
+            Workspace workspace)
         {
+            Validate.IsNotNull("sourceFile", sourceFile);
             Validate.IsNotNull("foundSymbol", foundSymbol);
+            Validate.IsNotNull("workspace", workspace);
+
+            ScriptFile[] referencedFiles =
+                workspace.ExpandScriptReferences(
+                    sourceFile);
 
             // look through the referenced files until definition is found
             // or there are no more file to look through
@@ -233,7 +239,11 @@ public GetDefinitionResult GetDefinitionOfSymbol(
             if (foundDefinition == null)
             {
                 CommandInfo cmdInfo = GetCommandInfo(foundSymbol.SymbolName);
-                foundDefinition = FindDeclarationForBuiltinCommand(cmdInfo, foundSymbol);
+                foundDefinition = 
+                    FindDeclarationForBuiltinCommand(
+                        cmdInfo, 
+                        foundSymbol,
+                        workspace);
             }
 
             return new GetDefinitionResult(foundDefinition);
@@ -334,7 +344,9 @@ private CommandInfo GetCommandInfo(string commandName)
             return commandInfo;
         }
 
-        private ScriptFile[] GetBuiltinCommandScriptFiles(PSModuleInfo moduleInfo)
+        private ScriptFile[] GetBuiltinCommandScriptFiles(
+            PSModuleInfo moduleInfo,
+            Workspace workspace)
         {
             if (moduleInfo != null)
             {
@@ -344,25 +356,20 @@ private ScriptFile[] GetBuiltinCommandScriptFiles(PSModuleInfo moduleInfo)
 
                 if (modPath.EndsWith(@".ps1") || modPath.EndsWith(@".psm1"))
                 {
-                    using (StreamReader streamReader = new StreamReader(modPath, Encoding.UTF8))
-                    {
-                        newFile = new ScriptFile(modPath, streamReader);
-                        scriptFiles.Add(newFile);
-                    }
+                    newFile = workspace.GetFile(modPath);
+                    newFile.IsAnalysisEnabled = false;
+                    scriptFiles.Add(newFile);
                 }
                 if (moduleInfo.NestedModules.Count > 0)
                 {
-                    string nestedModPath;
                     foreach (PSModuleInfo nestedInfo in moduleInfo.NestedModules)
                     {
-                        nestedModPath = nestedInfo.Path;
+                        string nestedModPath = nestedInfo.Path;
                         if (nestedModPath.EndsWith(@".ps1") || nestedModPath.EndsWith(@".psm1"))
                         {
-                            using (StreamReader streamReader = new StreamReader(nestedModPath, Encoding.UTF8))
-                            {
-                                newFile = new ScriptFile(nestedModPath, streamReader);
-                                scriptFiles.Add(newFile);
-                            }
+                            newFile = workspace.GetFile(nestedModPath);
+                            newFile.IsAnalysisEnabled = false;
+                            scriptFiles.Add(newFile);
                         }
                     }
                 }
@@ -373,7 +380,10 @@ private ScriptFile[] GetBuiltinCommandScriptFiles(PSModuleInfo moduleInfo)
             return new List<ScriptFile>().ToArray();
         }
 
-        private SymbolReference FindDeclarationForBuiltinCommand(CommandInfo cmdInfo, SymbolReference foundSymbol)
+        private SymbolReference FindDeclarationForBuiltinCommand(
+            CommandInfo cmdInfo, 
+            SymbolReference foundSymbol,
+            Workspace workspace)
         {
             SymbolReference foundDefinition = null;
             if (cmdInfo != null)
@@ -382,7 +392,9 @@ private SymbolReference FindDeclarationForBuiltinCommand(CommandInfo cmdInfo, Sy
                 ScriptFile[] nestedModuleFiles;
 
                 nestedModuleFiles =
-                    GetBuiltinCommandScriptFiles(GetCommandInfo(foundSymbol.SymbolName).Module);
+                    GetBuiltinCommandScriptFiles(
+                        GetCommandInfo(foundSymbol.SymbolName).Module,
+                        workspace);
 
                 while (foundDefinition == null && index < nestedModuleFiles.Length)
                 {
diff --git a/src/PowerShellEditorServices/Language/SymbolReference.cs b/src/PowerShellEditorServices/Language/SymbolReference.cs
@@ -59,13 +59,14 @@ public class SymbolReference
         /// Gets the contents of the line the given symbol is on
         /// </summary>
         public string SourceLine { get; internal set; }
-        #endregion
 
         /// <summary>
-        /// Gets the Filepath of the symbol
+        /// Gets the path of the file in which the symbol was found.
         /// </summary>
         public string FilePath { get; internal set; }
 
+        #endregion
+
         /// <summary>
         /// Constructs and instance of a SymbolReference
         /// </summary>
diff --git a/src/PowerShellEditorServices/Session/ScriptFile.cs b/src/PowerShellEditorServices/Session/ScriptFile.cs
@@ -32,6 +32,13 @@ public class ScriptFile
         /// </summary>
         public string FilePath { get; private set; }
 
+        /// <summary>
+        /// Gets or sets a boolean that determines whether
+        /// semantic analysis should be enabled for this file.
+        /// For internal use only.
+        /// </summary>
+        internal bool IsAnalysisEnabled { get; set; }
+
         /// <summary>
         /// Gets a string containing the full contents of the file.
         /// </summary>
@@ -101,6 +108,8 @@ public string[] ReferencedFiles
         public ScriptFile(string filePath, TextReader textReader)
         {
             this.FilePath = filePath;
+            this.IsAnalysisEnabled = true;
+
             this.ReadFile(textReader);
         }
 
diff --git a/src/PowerShellEditorServices/Session/Workspace.cs b/src/PowerShellEditorServices/Session/Workspace.cs
diff --git a/test/PowerShellEditorServices.Test/Language/LanguageServiceTests.cs b/test/PowerShellEditorServices.Test/Language/LanguageServiceTests.cs

Original file line number	Diff line number	Diff line change
`@@ -26,8 +26,9 @@ public override void ProcessMessage(`
`26`	`26`
`27`	`27`	`GetDefinitionResult definition =`
`28`	`28`	`editorSession.LanguageService.GetDefinitionOfSymbol(`
	`29`	`+ scriptFile,`
`29`	`30`	`foundSymbol,`
`30`		`- editorSession.Workspace.ExpandScriptReferences(scriptFile));`
	`31`	`+ editorSession.Workspace);`
`31`	`32`
`32`	`33`	`if (definition != null)`
`33`	`34`	`{`
Original file line number	Diff line number	Diff line change
`@@ -13,20 +13,9 @@ public abstract class FileRequest<TArguments> : RequestBase<TArguments>`
`13`	`13`	`{`
`14`	`14`	`protected ScriptFile GetScriptFile(EditorSession editorSession)`
`15`	`15`	`{`
`16`		`- ScriptFile scriptFile = null;`
`17`		`-`
`18`		`- if(!editorSession.Workspace.TryGetFile(`
`19`		`- this.Arguments.File,`
`20`		`- out scriptFile))`
`21`		`- {`
`22`		`- // TODO: Throw an exception that the message loop can create a response out of`
`23`		`-`
`24`		`- throw new FileNotFoundException(`
`25`		`- "A ScriptFile with the following path was not found in the EditorSession: {0}",`
	`16`	`+ return`
	`17`	`+ editorSession.Workspace.GetFile(`
`26`	`18`	`this.Arguments.File);`
`27`		`- }`
`28`		`-`
`29`		`- return scriptFile;`
`30`	`19`	`}`
`31`	`20`	`}`
`32`	`21`
Original file line number	Diff line number	Diff line change
`@@ -26,14 +26,8 @@ public override void ProcessMessage(`
`26`	`26`	`EditorSession editorSession,`
`27`	`27`	`MessageWriter messageWriter)`
`28`	`28`	`{`
`29`		`- ScriptFile scriptFile = null;`
`30`		`-`
`31`		`- // Only load the file if it isn't loaded already`
`32`		`- if (!editorSession.Workspace.TryGetFile(this.Arguments.File, out scriptFile))`
`33`		`- {`
`34`		`- // Open the file in the current session`
`35`		`- editorSession.Workspace.OpenFile(this.Arguments.File);`
`36`		`- }`
	`29`	`+ // Open the file in the current session`
	`30`	`+ editorSession.Workspace.GetFile(this.Arguments.File);`
`37`	`31`	`}`
`38`	`32`	`}`
`39`	`33`	`}`