Skip to content

Commit

Permalink
More simplification and dead code elimination
Browse files Browse the repository at this point in the history
  • Loading branch information
andyleejordan committed Jan 12, 2023
1 parent 193405a commit 70bcd04
Show file tree
Hide file tree
Showing 14 changed files with 51 additions and 213 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using Microsoft.PowerShell.EditorServices.Services;
using Microsoft.PowerShell.EditorServices.Services.Symbols;
using Microsoft.PowerShell.EditorServices.Services.TextDocument;
using Microsoft.PowerShell.EditorServices.Utility;
using Newtonsoft.Json.Linq;
using OmniSharp.Extensions.LanguageServer.Protocol.Models;
using OmniSharp.Extensions.LanguageServer.Protocol.Serialization;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
using Microsoft.PowerShell.EditorServices.Services;
using Microsoft.PowerShell.EditorServices.Services.Symbols;
using Microsoft.PowerShell.EditorServices.Services.TextDocument;
using Microsoft.PowerShell.EditorServices.Utility;
using Newtonsoft.Json.Linq;
using OmniSharp.Extensions.LanguageServer.Protocol;
using OmniSharp.Extensions.LanguageServer.Protocol.Models;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ internal enum PesterCommandType
/// Provides a specialization of SymbolReference containing
/// extra information about Pester test symbols.
/// </summary>
internal class PesterSymbolReference : SymbolReference
internal record PesterSymbolReference : SymbolReference
{
/// <summary>
/// Lookup for Pester keywords we support. Ideally we could extract these from Pester itself
Expand Down
11 changes: 1 addition & 10 deletions src/PowerShellEditorServices/Services/Symbols/ReferenceTable.cs
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,6 @@ public override AstVisitAction VisitCommand(CommandAst commandAst)
return AstVisitAction.Continue;
}

// TODO: We should examine if we really want to constrain the extents to the name only. This
// means that highlighting only highlights the symbol name, but providing the whole extend
// means the whole function (or class etc.) gets highlighted, which seems to be a personal
// preference.
public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst functionDefinitionAst)
{
// Extent for constructors and method trigger both this and VisitFunctionMember(). Covered in the latter.
Expand All @@ -122,7 +118,7 @@ public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst fun
return AstVisitAction.Continue;
}

// We only want the function name
// We only want the function name as the extent for highlighting (and so forth).
IScriptExtent nameExtent = VisitorUtils.GetNameExtent(functionDefinitionAst);
_references.AddReference(
SymbolType.Function,
Expand Down Expand Up @@ -159,7 +155,6 @@ public override AstVisitAction VisitTypeDefinition(TypeDefinitionAst typeDefinit
{
SymbolType symbolType = typeDefinitionAst.IsEnum ? SymbolType.Enum : SymbolType.Class;

// We only want the type name. Get start-location for name
IScriptExtent nameExtent = VisitorUtils.GetNameExtent(typeDefinitionAst);
_references.AddReference(symbolType, typeDefinitionAst.Name, nameExtent);

Expand Down Expand Up @@ -189,7 +184,6 @@ public override AstVisitAction VisitFunctionMember(FunctionMemberAst functionMem
? SymbolType.Constructor
: SymbolType.Method;

// We only want the method/ctor name. Get start-location for name
IScriptExtent nameExtent = VisitorUtils.GetNameExtent(functionMemberAst, true, false);
_references.AddReference(
symbolType,
Expand All @@ -201,7 +195,6 @@ public override AstVisitAction VisitFunctionMember(FunctionMemberAst functionMem

public override AstVisitAction VisitPropertyMember(PropertyMemberAst propertyMemberAst)
{
// We only want the property name. Get start-location for name
IScriptExtent nameExtent = VisitorUtils.GetNameExtent(propertyMemberAst, false);
_references.AddReference(
SymbolType.Property,
Expand All @@ -210,7 +203,5 @@ public override AstVisitAction VisitPropertyMember(PropertyMemberAst propertyMem

return AstVisitAction.Continue;
}

// TODO: What else can we implement?
}
}
62 changes: 5 additions & 57 deletions src/PowerShellEditorServices/Services/Symbols/SymbolReference.cs
Original file line number Diff line number Diff line change
@@ -1,75 +1,30 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

#nullable enable

using System.Diagnostics;
using System.Management.Automation.Language;
using Microsoft.PowerShell.EditorServices.Services.TextDocument;

namespace Microsoft.PowerShell.EditorServices.Services.Symbols
{
internal interface ISymbolReference
{
/// <summary>
/// Gets the symbol's type
/// </summary>
SymbolType SymbolType { get; }

/// <summary>
/// Gets the name of the symbol
/// </summary>
string SymbolName { get; }

/// <summary>
/// Gets the script extent of the symbol
/// </summary>
ScriptRegion ScriptRegion { get; }

/// <summary>
/// Gets the contents of the line the given symbol is on
/// </summary>
string SourceLine { get; }

/// <summary>
/// Gets the path of the file in which the symbol was found.
/// </summary>
string FilePath { get; }
}

/// <summary>
/// A class that holds the type, name, script extent, and source line of a symbol
/// </summary>
[DebuggerDisplay("SymbolType = {SymbolType}, SymbolName = {SymbolName}")]
internal class SymbolReference : ISymbolReference
internal record SymbolReference
{
#region Properties

/// <summary>
/// Gets the symbol's type
/// </summary>
public SymbolType SymbolType { get; }

/// <summary>
/// Gets the name of the symbol
/// </summary>
public string SymbolName { get; }

/// <summary>
/// Gets the script extent of the symbol
/// </summary>
public ScriptRegion ScriptRegion { get; }

/// <summary>
/// Gets the contents of the line the given symbol is on
/// </summary>
public string SourceLine { get; internal set; }

/// <summary>
/// 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>
Expand All @@ -88,16 +43,9 @@ public SymbolReference(
// TODO: Verify params
SymbolType = symbolType;
SymbolName = symbolName;
ScriptRegion = ScriptRegion.Create(scriptExtent);
ScriptRegion = new(scriptExtent);
FilePath = filePath;
SourceLine = sourceLine;

// TODO: Make sure end column number usage is correct

// Build the display string
//this.DisplayString =
// string.Format(
// "{0} {1}")
}

public SymbolReference(
Expand All @@ -108,7 +56,7 @@ public SymbolReference(
{
SymbolType = symbolType;
SymbolName = symbolName;
ScriptRegion = ScriptRegion.Create(scriptExtent);
ScriptRegion = new(scriptExtent);
FilePath = file.FilePath;
try
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,72 +196,6 @@ public static SymbolReference FindDefinitionOfSymbol(
return declarationVisitor.FoundDeclaration;
}

/// <summary>
/// Finds all symbols in a script
/// </summary>
/// <param name="scriptAst">The abstract syntax tree of the given script</param>
/// <returns>A collection of SymbolReference objects</returns>
public static IEnumerable<SymbolReference> FindSymbolsInDocument(Ast scriptAst)
{
FindSymbolsVisitor findSymbolsVisitor = new();
scriptAst.Visit(findSymbolsVisitor);
return findSymbolsVisitor.SymbolReferences;
}

/// <summary>
/// Checks if a given ast represents the root node of a *.psd1 file.
/// </summary>
/// <param name="ast">The abstract syntax tree of the given script</param>
/// <returns>true if the AST represents a *.psd1 file, otherwise false</returns>
public static bool IsPowerShellDataFileAst(Ast ast)
{
// sometimes we don't have reliable access to the filename
// so we employ heuristics to check if the contents are
// part of a psd1 file.
return IsPowerShellDataFileAstNode(
new { Item = ast, Children = new List<dynamic>() },
new Type[] {
typeof(ScriptBlockAst),
typeof(NamedBlockAst),
typeof(PipelineAst),
typeof(CommandExpressionAst),
typeof(HashtableAst) },
0);
}

private static bool IsPowerShellDataFileAstNode(dynamic node, Type[] levelAstMap, int level)
{
dynamic levelAstTypeMatch = node.Item.GetType().Equals(levelAstMap[level]);
if (!levelAstTypeMatch)
{
return false;
}

if (level == levelAstMap.Length - 1)
{
return levelAstTypeMatch;
}

IEnumerable<Ast> astsFound = (node.Item as Ast)?.FindAll(a => a is not null, false);
if (astsFound != null)
{
foreach (Ast astFound in astsFound)
{
if (!astFound.Equals(node.Item)
&& node.Item.Equals(astFound.Parent)
&& IsPowerShellDataFileAstNode(
new { Item = astFound, Children = new List<dynamic>() },
levelAstMap,
level + 1))
{
return true;
}
}
}

return false;
}

/// <summary>
/// Finds all files dot sourced in a script
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public override async Task<CommandOrCodeActionContainer> Handle(CodeActionParams
{
Uri = request.TextDocument.Uri
},
Edits = new TextEditContainer(ScriptRegion.ToTextEdit(markerCorrection.Edit))
Edits = new TextEditContainer(markerCorrection.Edit.ToTextEdit())
}))
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public override async Task<LocationOrLocationLinks> Handle(DefinitionParams requ
new Location
{
Uri = DocumentUri.From(foundDefinition.FilePath),
Range = ScriptRegion.GetRangeFromScriptRegion(foundDefinition.ScriptRegion)
Range = foundDefinition.ScriptRegion.ToRange()
}));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,15 @@ public override Task<SymbolInformationOrDocumentSymbolContainer> Handle(Document
? foundSymbols
.Select(r =>
{
// TODO: This should be a DocumentSymbol now as SymbolInformation is deprecated.
return new SymbolInformationOrDocumentSymbol(new SymbolInformation
{
ContainerName = containerName,
Kind = SymbolTypeUtils.GetSymbolKind(r.SymbolType),
Location = new Location
{
Uri = DocumentUri.From(r.FilePath),
Range = ScriptRegion.GetRangeFromScriptRegion(r.ScriptRegion)
Range = r.ScriptRegion.ToRange()
},
Name = SymbolTypeUtils.GetDecoratedSymbolName(r)
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,10 @@ await _symbolsService.FindSymbolDetailsAtLocationAsync(
symbolInfo.Add(new MarkedString("markdown", symbolDetails.Documentation));
}

Range symbolRange = ScriptRegion.GetRangeFromScriptRegion(symbolDetails.SymbolReference.ScriptRegion);

return new Hover
{
Contents = new MarkedStringsOrMarkupContent(symbolInfo),
Range = symbolRange
Range = symbolDetails.SymbolReference.ScriptRegion.ToRange()
};
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ await _symbolsService.ScanForReferencesOfSymbol(
locations.Add(new Location
{
Uri = DocumentUri.From(foundReference.FilePath),
Range = ScriptRegion.GetRangeFromScriptRegion(foundReference.ScriptRegion)
Range = foundReference.ScriptRegion.ToRange()
});
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,19 @@ public enum ScriptFileMarkerLevel
/// <summary>
        /// Information: This warning is trivial, but may be useful. They are recommended by PowerShell best practice.
        /// </summary>
        Information = 0,
Information = 0,
        /// <summary>
        /// WARNING: This warning may cause a problem or does not follow PowerShell's recommended guidelines.
        /// </summary>
        Warning = 1,
Warning = 1,
        /// <summary>
        /// ERROR: This warning is likely to cause a problem or does not follow PowerShell's required guidelines.
        /// </summary>
        Error = 2,
Error = 2,
        /// <summary>
        /// ERROR: This diagnostic is caused by an actual parsing error, and is generated only by the engine.
        /// </summary>
        ParseError = 3
ParseError = 3
};

/// <summary>
Expand Down Expand Up @@ -102,7 +102,7 @@ internal static ScriptFileMarker FromParseError(
{
Message = parseError.Message,
Level = ScriptFileMarkerLevel.Error,
ScriptRegion = ScriptRegion.Create(parseError.Extent),
ScriptRegion = new(parseError.Extent),
Source = "PowerShell"
};
}
Expand Down Expand Up @@ -157,7 +157,7 @@ internal static ScriptFileMarker FromDiagnosticRecord(PSObject psObject)
Message = diagnosticRecord.Message as string ?? string.Empty,
RuleName = diagnosticRecord.RuleName as string ?? string.Empty,
Level = level,
ScriptRegion = ScriptRegion.Create(diagnosticRecord.Extent as IScriptExtent),
ScriptRegion = new(diagnosticRecord.Extent as IScriptExtent),
Corrections = markerCorrections,
Source = "PSScriptAnalyzer"
};
Expand Down
Loading

0 comments on commit 70bcd04

Please sign in to comment.