diff --git a/CLAUDE.md b/CLAUDE.md index db2a1a95..efc2eb8a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -57,7 +57,7 @@ The projects wired together in `CSharpCodeAnalyst.sln` (the load-bearing ones): `DartExtractor/` is a standalone **Dart** package (not in the .NET solution) used by the Dart/Flutter import — see **Dart/Flutter import** below. -`TestSuite/` is a handcrafted C# solution used purely as parser input for the approval tests, and `TestSuiteDart/` is its Dart equivalent for the Dart import. Do not consume either from production code — they are intentionally full of odd language constructs. `ReferencedAssemblies/` contains the MSAGL DLLs referenced directly by `CSharpCodeAnalyst.csproj` and `Tests.csproj` (MSAGL is not on NuGet for the versions used here). +`TestSuite/` is a handcrafted C# solution used purely as parser input for the approval tests, and `TestSuiteDart/` is its Dart equivalent for the Dart import. Do not consume either from production code — they are intentionally full of odd language constructs. `TestSuiteGenerated/` is a third fixture solution, for the **source-generator** path only (`SourceGeneratorFixtureTests`): it uses `[GeneratedRegex]`, which ships with the SDK, so it needs no package reference and neither a restore nor a build — MSBuildWorkspace's design-time build runs the generator. None of the three is part of `CSharpCodeAnalyst.sln`. `ReferencedAssemblies/` contains the MSAGL DLLs referenced directly by `CSharpCodeAnalyst.csproj` and `Tests.csproj` (MSAGL is not on NuGet for the versions used here). ## Architectural notes worth knowing before editing @@ -93,7 +93,7 @@ Analyzers are the boxes under the **Analyzers** ribbon button. Each implements ` Data flow, end to end: 1. **Algorithm** in `CodeGraph/Algorithms/...` takes the `CodeGraph` and returns a plain result object. Type-level analyses lift relationships to the containing type, deduplicate, and exclude `IsExternal` nodes (see `TypeDependencyAnalysis` / `SystemMetricsAnalysis` as the reference); reuse `Type.IsDependency()` to decide which edges count. 2. **`Analyze`** runs the algorithm; on an empty result it calls `_userNotification.ShowSuccess(...NoData)` and returns, otherwise it builds a **table view model** and publishes `new ShowTabularDataRequest(Id, Name, vm)` on the message bus. -3. **Table VM** derives from `Table` (`AnalyzerSdk/DynamicDataGrid/Contracts/TabularData/`): `GetColumns()` returns `TableColumnDefinition`s (each binds a `PropertyName` on the row VM), `GetData()` returns the `TableRow`s. Optional: `CanFilter`/`Filter`, `GetCommands()` (context-menu / double-click actions), row-details template, and per-column `Rating` (an `IMetricRating` → colored cell background, see `ThresholdRating` and `RatingToBrushConverter`). +3. **Table VM** derives from `Table` (`AnalyzerSdk/DynamicDataGrid/Contracts/TabularData/`): `GetColumns()` returns `TableColumnDefinition`s (each binds a `PropertyName` on the row VM), `GetData()` returns the `TableRow`s. Optional: `CanFilter`/`Filter`, `GetCommands()` (context-menu / double-click actions), row-details template, and per-column `Rating` (an `IMetricRating` → colored cell background, see `ThresholdRating` and `RatingToBrushConverter`). **"Copy table as CSV" comes for free** — `DynamicDataGrid` appends it to every table's context menu and copies the rows currently on screen (after filtering and sorting) via `TableCsv`; no analyzer implements its own export. 4. **Row VM** derives from `TableRow` and exposes one property per column (plus a `SortMemberName`/`RatingValuePropertyName` numeric backer when the displayed column is a formatted string). 5. **Register** the analyzer in `CSharpCodeAnalyst/Features/Analyzers/AnalyzerManager.LoadAnalyzers` (add a `using = ...` alias and an `_analyzers.Add`). **No XAML change** is needed: the ribbon `RibbonSplitButton` binds `ItemsSource` to `MainViewModel.Analyzers` (= `AnalyzerManager.All`) and runs `ExecuteAnalyzerCommand` with the analyzer `Id`; `MainViewModel` publishes the result into a `DynamicTab` that hosts a `DynamicDataGrid`. 6. **Strings** live in `CSharpCodeAnalyst.Analyzers/Resources/Strings.resx` **and** its hand-maintained `Strings.Designer.cs` (add the getter yourself). Convention: `Analyzer__Label` / `_Tooltip` / `_NoData`, `Column__`. diff --git a/CSharpCodeAnalyst.AnalyzerSdk/DynamicDataGrid/Contracts/TabularData/TableCsv.cs b/CSharpCodeAnalyst.AnalyzerSdk/DynamicDataGrid/Contracts/TabularData/TableCsv.cs new file mode 100644 index 00000000..c2fa8682 --- /dev/null +++ b/CSharpCodeAnalyst.AnalyzerSdk/DynamicDataGrid/Contracts/TabularData/TableCsv.cs @@ -0,0 +1,87 @@ +using System.Globalization; +using System.Text; + +namespace CSharpCodeAnalyst.AnalyzerSdk.DynamicDataGrid.Contracts.TabularData; + +/// +/// Turns a table into CSV, so a result can be counted, sorted or pivoted somewhere else. Separate from +/// the grid that offers it, because the interesting part - quoting, and reading a cell the way the +/// column binds it - is worth testing on its own. +/// +public static class TableCsv +{ + /// The columns, in display order. Their PropertyName reads the cell. + /// + /// The rows to write. The caller passes what is on screen - filtered and sorted - rather than the + /// table's full data, because narrowing the result down is usually the first step. + /// + /// + /// Null takes the list separator of the current culture, which is what a spreadsheet on the same + /// machine expects: a comma in en-US, a semicolon in de-DE. With the wrong one the paste lands in a + /// single column. + /// + public static string Build(IEnumerable columns, IEnumerable rows, + string? separator = null) + { + ArgumentNullException.ThrowIfNull(columns); + ArgumentNullException.ThrowIfNull(rows); + + separator ??= CultureInfo.CurrentCulture.TextInfo.ListSeparator; + + var columnList = columns.ToList(); + var csv = new StringBuilder(); + + csv.AppendLine(string.Join(separator, columnList.Select(column => Escape(column.Header, separator)))); + + foreach (var row in rows) + { + csv.AppendLine(string.Join(separator, + columnList.Select(column => Escape(ReadCell(row, column.PropertyName), separator)))); + } + + return csv.ToString(); + } + + /// + /// Reads the property the column binds to. A column without a property name - a button, an image - + /// contributes an empty cell instead of breaking the row. + /// + private static string ReadCell(object row, string? propertyName) + { + if (string.IsNullOrEmpty(propertyName)) + { + return string.Empty; + } + + var value = row.GetType().GetProperty(propertyName)?.GetValue(row); + + return value switch + { + null => string.Empty, + + // The same culture the cell was rendered with, so a number reads the same in both places. + IFormattable formattable => formattable.ToString(null, CultureInfo.CurrentCulture), + _ => value.ToString() ?? string.Empty + }; + } + + /// + /// RFC 4180: quote a value that holds the separator, a quote or a line break, and double the quotes + /// inside it. Names and hints in this application contain all three. + /// + private static string Escape(string? value, string separator) + { + if (string.IsNullOrEmpty(value)) + { + return string.Empty; + } + + if (!value.Contains(separator, StringComparison.Ordinal) && !value.Contains('"') && + !value.Contains('\n') && !value.Contains('\r')) + { + return value; + } + + return $"\"{value.Replace("\"", "\"\"", StringComparison.Ordinal)}\""; + } +} diff --git a/CSharpCodeAnalyst.AnalyzerSdk/Search/SearchExpression.cs b/CSharpCodeAnalyst.AnalyzerSdk/Search/SearchExpression.cs index 72ba1c79..e3aac223 100644 --- a/CSharpCodeAnalyst.AnalyzerSdk/Search/SearchExpression.cs +++ b/CSharpCodeAnalyst.AnalyzerSdk/Search/SearchExpression.cs @@ -38,6 +38,12 @@ protected Term(string searchTerm) { SearchMode = SearchType.ExternalCode; } + else if (lowerSearchTerm is "source:generated") + { + // Code a tool wrote. Mostly useful negated ("-source:generated"), because a resx designer or + // the XAML markup compiler contributes rows nobody can act on. + SearchMode = SearchType.GeneratedCode; + } else { var (isPascalCase, regex) = PascalCaseSearch.CreateSearchRegex(searchTerm); @@ -88,7 +94,10 @@ internal enum SearchType FullNameResharperStyle, ExternalCode, - InternalCode + InternalCode, + + // Written by a tool rather than a person (CodeElement.IsGenerated). + GeneratedCode } internal class And : IExpression @@ -120,6 +129,31 @@ public bool Evaluate(CodeElement? item) return _conditions.Any(c => c.Evaluate(item)); } } + + /// + /// Negates a condition, so a search can exclude instead of select ("-Strings." hides everything + /// whose name contains "Strings."). + /// + /// An item without a code element never matches, not even a negated condition. Every term + /// answers "no" for a null item, and negation must not silently turn that into a match - the + /// tree has a virtual root without a code element that would otherwise light up on every + /// exclusion. + /// + /// + internal class Not : IExpression + { + private readonly IExpression _condition; + + public Not(IExpression condition) + { + _condition = condition; + } + + public bool Evaluate(CodeElement? item) + { + return item is not null && !_condition.Evaluate(item); + } + } } internal class FullNameSearch(string searchTerm) : Term(searchTerm) @@ -136,6 +170,7 @@ public override bool Evaluate(CodeElement? item) SearchType.Type => item.ElementType == Type, SearchType.InternalCode => !item.IsExternal, SearchType.ExternalCode => item.IsExternal, + SearchType.GeneratedCode => item.IsGenerated, SearchType.FullNameResharperStyle => Regex!.IsMatch(item.FullName), _ => item.FullName.Contains(SearchTerm, StringComparison.InvariantCultureIgnoreCase) }; @@ -156,6 +191,7 @@ public override bool Evaluate(CodeElement? item) SearchType.Type => item.ElementType == Type, SearchType.InternalCode => !item.IsExternal, SearchType.ExternalCode => item.IsExternal, + SearchType.GeneratedCode => item.IsGenerated, SearchType.FullNameResharperStyle => Regex!.IsMatch(item.Name), _ => item.Name.Contains(SearchTerm, StringComparison.InvariantCultureIgnoreCase) }; diff --git a/CSharpCodeAnalyst.AnalyzerSdk/Search/SearchExpressionFactory.cs b/CSharpCodeAnalyst.AnalyzerSdk/Search/SearchExpressionFactory.cs index 21d57a83..b01a620e 100644 --- a/CSharpCodeAnalyst.AnalyzerSdk/Search/SearchExpressionFactory.cs +++ b/CSharpCodeAnalyst.AnalyzerSdk/Search/SearchExpressionFactory.cs @@ -2,6 +2,13 @@ public static class SearchExpressionFactory { + /// + /// Marks a term as excluding rather than selecting. A minus sign cannot start a C# identifier, so + /// it is free to use here; imported graphs may contain one inside a name, but not at the start of + /// a search term. + /// + private const char NegationPrefix = '-'; + private static Term CreateTerm(string search, TextSearchField searchField) { if (searchField == TextSearchField.FullName) @@ -12,7 +19,29 @@ private static Term CreateTerm(string search, TextSearchField searchField) return new NameSearch(search); } - public static IExpression CreateSearchExpression(string searchText, TextSearchField searchField = TextSearchField.FullName) + /// + /// Wraps the term in a negation when it starts with '-'. The negation belongs to its own term, so + /// it binds tighter than the AND of a group and than the OR between groups: "-a b | c" reads as + /// "((NOT a) AND b) OR c". A lone '-' has nothing to negate and stays a literal search term. + /// + private static IExpression CreateTermOrNegation(string token, TextSearchField searchField, bool allowNegation) + { + if (allowNegation && token.Length > 1 && token[0] == NegationPrefix) + { + return new Term.Not(CreateTerm(token[1..], searchField)); + } + + return CreateTerm(token, searchField); + } + + /// + /// Whether a leading '-' excludes the term. Pass false where an expression that matches almost + /// everything is harmful rather than useful: the tree expands and highlights every ancestor of a + /// match, so an exclusion would unfold the whole tree at once. With negation off the '-' is part + /// of the search term like any other character. + /// + public static IExpression CreateSearchExpression(string searchText, + TextSearchField searchField = TextSearchField.FullName, bool allowNegation = true) { // Or binds less. var orTerms = searchText @@ -24,7 +53,7 @@ public static IExpression CreateSearchExpression(string searchText, TextSearchFi { var andExpressions = orTerm .Split([' '], StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries) - .Select(IExpression (t) => CreateTerm(t, searchField)) + .Select(t => CreateTermOrNegation(t, searchField, allowNegation)) .ToArray(); orExpressions.Add(new Term.And(andExpressions)); diff --git a/CSharpCodeAnalyst.Analyzers/DeadCode/Analyzer.cs b/CSharpCodeAnalyst.Analyzers/DeadCode/Analyzer.cs new file mode 100644 index 00000000..f6549255 --- /dev/null +++ b/CSharpCodeAnalyst.Analyzers/DeadCode/Analyzer.cs @@ -0,0 +1,69 @@ +using CSharpCodeAnalyst.Analyzers.DeadCode.Presentation; +using CSharpCodeAnalyst.Analyzers.Resources; +using CSharpCodeAnalyst.AnalyzerSdk.Contracts; +using CSharpCodeAnalyst.AnalyzerSdk.Messages; +using CSharpCodeAnalyst.AnalyzerSdk.Notifications; +using CSharpCodeAnalyst.CodeGraph.Algorithms.DeadCode; +using CSharpCodeAnalyst.CodeGraph.Declarations; + +namespace CSharpCodeAnalyst.Analyzers.DeadCode; + +/// +/// Lists the code nobody references any more - the topmost element of every dead subtree, together +/// with the hint that explains why it might still be alive. +/// +public class Analyzer : IAnalyzer +{ + private readonly ExternalContractStore _externalContracts; + private readonly IPublisher _messaging; + private readonly IUserNotification _userNotification; + + public Analyzer(IPublisher messaging, IUserNotification userNotification, + ExternalContractStore externalContracts) + { + _messaging = messaging; + _userNotification = userNotification; + _externalContracts = externalContracts; + } + + public string Id { get; } = "DeadCode"; + public string Name { get; } = Strings.Analyzer_DeadCode_Label; + public string Description { get; set; } = Strings.Analyzer_DeadCode_Tooltip; + + public void Analyze(CodeGraph.Graph.CodeGraph graph) + { + var findings = DeadCodeAnalysis.Calculate(graph, _externalContracts); + + if (findings.Count == 0) + { + _userNotification.ShowSuccess(Strings.Analyzer_DeadCode_NoData); + return; + } + + var vm = new DeadCodeViewModel(findings, _messaging); + _messaging.Publish(new ShowTabularDataRequest(Id, Name, vm)); + } + + public string? GetPersistentData() + { + // No configuration or state to persist. + return null; + } + + public void SetPersistentData(string? data) + { + // No configuration or state to persist. + } + + public bool IsDirty() + { + return false; + } + + public event EventHandler? DataChanged; + + protected virtual void OnDataChanged() + { + DataChanged?.Invoke(this, EventArgs.Empty); + } +} diff --git a/CSharpCodeAnalyst.Analyzers/DeadCode/Presentation/DeadCodeRowViewModel.cs b/CSharpCodeAnalyst.Analyzers/DeadCode/Presentation/DeadCodeRowViewModel.cs new file mode 100644 index 00000000..399ab2c6 --- /dev/null +++ b/CSharpCodeAnalyst.Analyzers/DeadCode/Presentation/DeadCodeRowViewModel.cs @@ -0,0 +1,114 @@ +using CSharpCodeAnalyst.Analyzers.Resources; +using CSharpCodeAnalyst.AnalyzerSdk.DynamicDataGrid.Contracts.TabularData; +using CSharpCodeAnalyst.CodeGraph.Algorithms.DeadCode; +using CSharpCodeAnalyst.CodeGraph.Graph; + +namespace CSharpCodeAnalyst.Analyzers.DeadCode.Presentation; + +public class DeadCodeRowViewModel : TableRow +{ + /// Beyond this many related members the hint only states the count - the cell has to stay readable. + private const int MaxNamedRelatedMembers = 3; + + internal DeadCodeRowViewModel(DeadCodeFinding finding) + { + Element = finding.Element; + Name = finding.Element.FullName; + Kind = finding.Element.ElementType.ToString(); + // Fully qualified: WPF pulls a global "Accessibility" namespace into scope; ours is AccessLevel. + Access = finding.Element.AccessLevel == CodeGraph.Graph.AccessLevel.Unknown + ? string.Empty + : finding.Element.AccessLevel.ToString(); + + Confidence = finding.Confidence.ToString(); + + // Bound for the colour rating and for sorting; the column displays the word. + ConfidenceValue = (int)finding.Confidence; + Hint = FormatHint(finding); + } + + /// The underlying graph node, used to jump to the source and to add it to the Code Explorer. + public CodeElement Element { get; } + + public string Name { get; } + public string Kind { get; } + + /// The element's visibility, empty when the producer did not supply one. + public string Access { get; } + + public string Confidence { get; } + + /// Numeric backer of for the colour rating and for sorting. + public int ConfidenceValue { get; } + + /// + /// Two kinds of note, joined into one cell: why the element might be alive despite having no + /// visible reference (entry point, test code, attributes), and - for a contract finding - what + /// dies together with it. Empty means neither applies, so nothing speaks against deleting it. + /// + public string Hint { get; } + + private static string FormatHint(DeadCodeFinding finding) + { + var parts = new List(); + + if (finding.Hints.HasFlag(DeadCodeHint.EntryPoint)) + { + parts.Add(Strings.DeadCode_Hint_EntryPoint); + } + + if (finding.Hints.HasFlag(DeadCodeHint.TestCode)) + { + parts.Add(Strings.DeadCode_Hint_TestCode); + } + + if (finding.Hints.HasFlag(DeadCodeHint.Generated)) + { + parts.Add(Strings.DeadCode_Hint_Generated); + } + + if (finding.Hints.HasFlag(DeadCodeHint.UsedOnlyByTests)) + { + parts.Add(finding.TestReferences.Count == 0 + ? Strings.DeadCode_Hint_UsedOnlyByTestsWithoutCaller + : string.Format(Strings.DeadCode_Hint_UsedOnlyByTests, Format(finding.TestReferences))); + } + + if (finding.Hints.HasFlag(DeadCodeHint.ContractNeverCalled)) + { + parts.Add(string.Format(Strings.DeadCode_Hint_ContractNeverCalled, FormatRelated(finding))); + } + + if (finding.Hints.HasFlag(DeadCodeHint.ImplementsDeadContract)) + { + parts.Add(string.Format(Strings.DeadCode_Hint_ImplementsDeadContract, FormatRelated(finding))); + } + + if (finding.Hints.HasFlag(DeadCodeHint.ImplementsExternalContract)) + { + parts.Add(string.Format(Strings.DeadCode_Hint_ImplementsExternalContract, finding.ExternalContract)); + } + + if (finding.Hints.HasFlag(DeadCodeHint.Attributed)) + { + parts.Add(string.Format(Strings.DeadCode_Hint_Attributed, string.Join(", ", finding.Attributes))); + } + + return string.Join("; ", parts); + } + + private static string FormatRelated(DeadCodeFinding finding) + { + return Format(finding.RelatedMembers); + } + + private static string Format(IReadOnlyList members) + { + if (members.Count > MaxNamedRelatedMembers) + { + return string.Format(Strings.DeadCode_Hint_RelatedCount, members.Count); + } + + return string.Join(", ", members.Select(m => m.FullName)); + } +} diff --git a/CSharpCodeAnalyst.Analyzers/DeadCode/Presentation/DeadCodeViewModel.cs b/CSharpCodeAnalyst.Analyzers/DeadCode/Presentation/DeadCodeViewModel.cs new file mode 100644 index 00000000..6c998803 --- /dev/null +++ b/CSharpCodeAnalyst.Analyzers/DeadCode/Presentation/DeadCodeViewModel.cs @@ -0,0 +1,141 @@ +using System.Collections.ObjectModel; +using System.Windows; +using CSharpCodeAnalyst.Analyzers.Resources; +using CSharpCodeAnalyst.AnalyzerSdk.Contracts; +using CSharpCodeAnalyst.AnalyzerSdk.DynamicDataGrid.Contracts.TabularData; +using CSharpCodeAnalyst.AnalyzerSdk.Messages; +using CSharpCodeAnalyst.AnalyzerSdk.Search; +using CSharpCodeAnalyst.AnalyzerSdk.Wpf; +using CSharpCodeAnalyst.CodeGraph.Algorithms.DeadCode; + +namespace CSharpCodeAnalyst.Analyzers.DeadCode.Presentation; + +internal class DeadCodeViewModel : Table +{ + private readonly IPublisher _messaging; + private readonly ObservableCollection _rows; + + internal DeadCodeViewModel(List findings, IPublisher messaging) + { + _messaging = messaging; + + // Highest confidence first: that is the part of the result you can work through without checking + // every entry by hand, so it belongs at the top before anyone touches a column header. The + // analysis already sorts by name, which stays the tie breaker within a confidence band. + var rows = findings + .OrderByDescending(f => f.Confidence) + .Select(f => new DeadCodeRowViewModel(f)); + _rows = new ObservableCollection(rows); + } + + public override bool CanFilter => true; + + public override IEnumerable GetColumns() + { + return new List + { + new() + { + Type = ColumnType.Text, + Header = Strings.Column_DeadCode_Element, + PropertyName = nameof(DeadCodeRowViewModel.Name) + }, + new() + { + Type = ColumnType.Text, + Header = Strings.Column_DeadCode_Kind, + PropertyName = nameof(DeadCodeRowViewModel.Kind), + Width = 90 + }, + new() + { + Type = ColumnType.Text, + Header = Strings.Column_DeadCode_Access, + PropertyName = nameof(DeadCodeRowViewModel.Access), + Width = 80 + }, + new() + { + Type = ColumnType.Text, + Header = Strings.Column_DeadCode_Confidence, + PropertyName = nameof(DeadCodeRowViewModel.Confidence), + Width = 80, + + // High (2) green, Medium (1) orange, Low (0) red - here a larger value is better. + Rating = new ThresholdRating(2, 1, false), + RatingValuePropertyName = nameof(DeadCodeRowViewModel.ConfidenceValue), + SortMemberName = nameof(DeadCodeRowViewModel.ConfidenceValue) + }, + new() + { + // Carries both the doubts (entry point, test code, attributes) and the explanation of a + // contract finding. Empty means nothing speaks against deleting the element, and sorting + // brings those rows together. + Type = ColumnType.Text, + Header = Strings.Column_DeadCode_Hint, + PropertyName = nameof(DeadCodeRowViewModel.Hint) + } + }; + } + + public override ObservableCollection GetData() + { + return _rows; + } + + /// + /// Filters by element name using the same search expression as the Advanced Search (camel-case, + /// OR via '|', AND via spaces, exclusion via a leading '-'). Exclusion is what makes a long result + /// usable: "-Strings. -Tests" drops whole groups of findings at once. + /// + public override ObservableCollection Filter(string searchText) + { + if (string.IsNullOrWhiteSpace(searchText)) + { + return _rows; + } + + var expression = SearchExpressionFactory.CreateSearchExpression(searchText); + var filtered = _rows + .Cast() + .Where(row => expression.Evaluate(row.Element)); + return new ObservableCollection(filtered); + } + + public override DataTemplate? GetRowDetailsTemplate() + { + return null; + } + + public override List GetCommands() + { + return + [ + new CommandDefinition + { + Header = Strings.JumpToCode, + Command = new WpfCommand(JumpToCode, CanJumpToCode) + }, + new CommandDefinition + { + Header = Strings.CopyToExplorerGraph_MenuItem, + Command = new WpfCommand(ShowInExplorer) + } + ]; + } + + private void ShowInExplorer(DeadCodeRowViewModel row) + { + _messaging.Publish(new AddNodeToGraphRequest(row.Element)); + } + + private static bool CanJumpToCode(DeadCodeRowViewModel row) + { + return row.Element.SourceLocations.Count > 0; + } + + private void JumpToCode(DeadCodeRowViewModel row) + { + _messaging.Publish(new OpenSourceLocationRequest(row.Element.SourceLocations[0])); + } +} diff --git a/CSharpCodeAnalyst.Analyzers/Resources/Strings.Designer.cs b/CSharpCodeAnalyst.Analyzers/Resources/Strings.Designer.cs index cd584a66..d591af2c 100644 --- a/CSharpCodeAnalyst.Analyzers/Resources/Strings.Designer.cs +++ b/CSharpCodeAnalyst.Analyzers/Resources/Strings.Designer.cs @@ -257,6 +257,168 @@ public static string Analyzer_SystemMetrics_Tooltip { } } + /// + /// Looks up a localized string similar to Dead Code. + /// + public static string Analyzer_DeadCode_Label { + get { + return ResourceManager.GetString("Analyzer_DeadCode_Label", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to No unreferenced elements found. + /// + public static string Analyzer_DeadCode_NoData { + get { + return ResourceManager.GetString("Analyzer_DeadCode_NoData", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Finds elements nothing references any more. Only the topmost element of a dead subtree is listed; the hint column marks the cases that may still be used through XAML, reflection or a test runner.. + /// + public static string Analyzer_DeadCode_Tooltip { + get { + return ResourceManager.GetString("Analyzer_DeadCode_Tooltip", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Element. + /// + public static string Column_DeadCode_Element { + get { + return ResourceManager.GetString("Column_DeadCode_Element", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Notes. + /// + public static string Column_DeadCode_Hint { + get { + return ResourceManager.GetString("Column_DeadCode_Hint", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Kind. + /// + public static string Column_DeadCode_Kind { + get { + return ResourceManager.GetString("Column_DeadCode_Kind", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Access. + /// + public static string Column_DeadCode_Access { + get { + return ResourceManager.GetString("Column_DeadCode_Access", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Confidence. + /// + public static string Column_DeadCode_Confidence { + get { + return ResourceManager.GetString("Column_DeadCode_Confidence", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Attributes: {0}. + /// + public static string DeadCode_Hint_Attributed { + get { + return ResourceManager.GetString("DeadCode_Hint_Attributed", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Implemented but never called: {0}. + /// + public static string DeadCode_Hint_ContractNeverCalled { + get { + return ResourceManager.GetString("DeadCode_Hint_ContractNeverCalled", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Entry point. + /// + public static string DeadCode_Hint_EntryPoint { + get { + return ResourceManager.GetString("DeadCode_Hint_EntryPoint", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Implements unused contract: {0}. + /// + public static string DeadCode_Hint_ImplementsDeadContract { + get { + return ResourceManager.GetString("DeadCode_Hint_ImplementsDeadContract", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Implements external contract: {0}. + /// + public static string DeadCode_Hint_ImplementsExternalContract { + get { + return ResourceManager.GetString("DeadCode_Hint_ImplementsExternalContract", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to {0} members. + /// + public static string DeadCode_Hint_RelatedCount { + get { + return ResourceManager.GetString("DeadCode_Hint_RelatedCount", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Test code. + /// + public static string DeadCode_Hint_TestCode { + get { + return ResourceManager.GetString("DeadCode_Hint_TestCode", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Used only by tests: {0}. + /// + public static string DeadCode_Hint_UsedOnlyByTests { + get { + return ResourceManager.GetString("DeadCode_Hint_UsedOnlyByTests", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Used only by tests. + /// + public static string DeadCode_Hint_UsedOnlyByTestsWithoutCaller { + get { + return ResourceManager.GetString("DeadCode_Hint_UsedOnlyByTestsWithoutCaller", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Generated code. + /// + public static string DeadCode_Hint_Generated { + get { + return ResourceManager.GetString("DeadCode_Hint_Generated", resourceCulture); + } + } + /// /// Looks up a localized string similar to Type Cohesion. /// diff --git a/CSharpCodeAnalyst.Analyzers/Resources/Strings.resx b/CSharpCodeAnalyst.Analyzers/Resources/Strings.resx index 1762903e..0809af49 100644 --- a/CSharpCodeAnalyst.Analyzers/Resources/Strings.resx +++ b/CSharpCodeAnalyst.Analyzers/Resources/Strings.resx @@ -1,4 +1,4 @@ - + + + all + compile; build; native; contentfiles; analyzers; buildtransitive + all compile; build; native; contentfiles; analyzers; buildtransitive diff --git a/CSharpCodeAnalyst.CodeParser/Parser/Config/ParserConfig.cs b/CSharpCodeAnalyst.CodeParser/Parser/Config/ParserConfig.cs index 6cb505f5..2865e3dd 100644 --- a/CSharpCodeAnalyst.CodeParser/Parser/Config/ParserConfig.cs +++ b/CSharpCodeAnalyst.CodeParser/Parser/Config/ParserConfig.cs @@ -5,23 +5,21 @@ public class ParserConfig private readonly ProjectExclusionRegExCollection _projectExclusionFilters; public ParserConfig(ProjectExclusionRegExCollection projectExclusionFilters, bool includeExternals, - bool includeGeneratedCode = false, bool splitPropertyAccessors = false) + bool splitPropertyAccessors = false, bool includeXamlReferences = true) { _projectExclusionFilters = projectExclusionFilters; IncludeExternals = includeExternals; - IncludeGeneratedCode = includeGeneratedCode; SplitPropertyAccessors = splitPropertyAccessors; + IncludeXamlReferences = includeXamlReferences; } public bool IncludeExternals { get; } - /// - /// When enabled, source-generated documents (e.g. CommunityToolkit.Mvvm [ObservableProperty] / - /// [RelayCommand], [GeneratedRegex], ...) are included in phase 1 so the generated members get - /// their own code element instead of being collapsed onto the containing type via the phase-2 - /// fallback. - /// - public bool IncludeGeneratedCode { get; } + // There is no "include generated code" option. Generated code is always parsed: leaving it out + // removes the only reference many hand-written elements have (the markup compiler's Connect is the + // sole caller of every XAML event handler, an [ObservableProperty] the only reader of its backing + // field), which turns them into dead code. What a tool wrote carries CodeElement.IsGenerated instead, + // so a result can leave it out without the graph losing an edge. /// /// When enabled, each property is split into its getter and setter accessor as separate child @@ -31,6 +29,14 @@ public ParserConfig(ProjectExclusionRegExCollection projectExclusionFilters, boo /// public bool SplitPropertyAccessors { get; } + /// + /// When enabled, the XAML files next to the analyzed projects are scanned for the references the + /// markup compiler does not turn into C# (element tags, {x:Static}, {x:Type}) and + /// those become relationships in the graph. Without it a control that is only instantiated from + /// XAML looks unreferenced. + /// + public bool IncludeXamlReferences { get; } + public bool IsProjectIncluded(string projectName) { diff --git a/CSharpCodeAnalyst.CodeParser/Parser/DeclarationAnalyzer.cs b/CSharpCodeAnalyst.CodeParser/Parser/DeclarationAnalyzer.cs index c14249bd..645d7733 100644 --- a/CSharpCodeAnalyst.CodeParser/Parser/DeclarationAnalyzer.cs +++ b/CSharpCodeAnalyst.CodeParser/Parser/DeclarationAnalyzer.cs @@ -1,4 +1,5 @@ using System.Diagnostics; +using CSharpCodeAnalyst.CodeGraph.Declarations; using CSharpCodeAnalyst.CodeGraph.Graph; using CSharpCodeAnalyst.CodeParser.Parser.Config; using Microsoft.CodeAnalysis; @@ -20,14 +21,16 @@ internal class DeclarationAnalyzer private readonly SyntaxNodeAnalyzer _bodyAnalyzer; private readonly RelationshipBuilder _builder; private readonly ParserConfig _config; + private readonly ExternalContractStore _externalContracts; internal DeclarationAnalyzer(RelationshipBuilder builder, SyntaxNodeAnalyzer bodyAnalyzer, Artifacts artifacts, - ParserConfig config) + ParserConfig config, ExternalContractStore externalContracts) { _builder = builder; _bodyAnalyzer = bodyAnalyzer; _artifacts = artifacts; _config = config; + _externalContracts = externalContracts; } /// @@ -50,6 +53,8 @@ public void Analyze(Solution solution, CodeElement element, ISymbol symbol) AnalyzeInheritanceRelationships(element, typeSymbol); AnalyzeEnumMemberInitializers(solution, element, typeSymbol); AnalyzePrimaryConstructorBaseArguments(solution, element, typeSymbol); + RecordExternalInterfaceImplementations(typeSymbol); + RecordIfNotifyingType(element, typeSymbol); } else if (symbol is IMethodSymbol methodSymbol) { @@ -390,6 +395,91 @@ private void AddMethodOverrideRelationship(CodeElement sourceElement, IMethodSym // Maybe we override a framework method. Happens also if the base method is a generic one. // In this case the GetSymbolKey is different. One uses T, the overriding method uses the actual type. _builder.AddRelationshipWithFallbackToContainingType(sourceElement, methodSymbol, RelationshipType.Overrides, locations, RelationshipAttribute.None); + + RecordIfExternalContract(sourceElement, methodSymbol); + } + + /// + /// An override whose base member lives outside the analyzed code produces no relationship at all - + /// there is no element to point at - so the member ends up without a single incoming reference and + /// looks like dead code. The fact is recorded beside the graph instead. + /// The containing type decides: when it is one of ours the contract is internal, and the + /// edge already expresses it. + /// + private void RecordIfExternalContract(CodeElement sourceElement, ISymbol contractMember) + { + var containingType = contractMember.ContainingType; + if (containingType is null || _builder.FindInternalCodeElement(containingType.OriginalDefinition) is not null) + { + return; + } + + _externalContracts.Add(sourceElement.Id, $"{containingType.Name}.{contractMember.Name}"); + } + + /// + /// Members that implement an interface from outside the analyzed code (ICommand.Execute, + /// IValueConverter.Convert, ...). Nothing in the graph shows this: the interface is not an + /// element, and the implementation is called by the framework, never from our code. + /// + /// Only foreign interfaces are scanned. For our own, + /// creates real edges from the interface side. + /// + /// + /// The interfaces come from and are therefore + /// already constructed, so + /// can be called directly - the mapping trap described at + /// does not apply here. + /// + /// + private void RecordExternalInterfaceImplementations(INamedTypeSymbol typeSymbol) + { + foreach (var contract in typeSymbol.AllInterfaces) + { + // A constructed generic interface (IHandler) is not in the map - the definition is. + if (_builder.FindInternalCodeElement(contract.OriginalDefinition) is not null) + { + continue; + } + + foreach (var contractMember in contract.GetMembers()) + { + var implementation = typeSymbol.FindImplementationForInterfaceMember(contractMember); + if (implementation is null) + { + continue; + } + + var element = _builder.FindInternalCodeElement(implementation) + ?? _builder.FindInternalCodeElement(implementation.OriginalDefinition); + if (element is not null) + { + _externalContracts.Add(element.Id, $"{contract.Name}.{contractMember.Name}"); + } + } + } + } + + /// + /// Records a type that raises change notifications - INotifyPropertyChanged appears + /// anywhere in its interface set, no matter which class of the inheritance chain implements it. + /// The member-level contract cannot carry this fact when the implementation sits in a base class + /// outside the analyzed code (ObservableObject, BindableBase, ...): the derived type then has no + /// PropertyChanged member of its own, so from the graph alone nothing says it is a view model, + /// and the dead code analysis would rate its bindable properties too confidently. + /// + private void RecordIfNotifyingType(CodeElement element, INamedTypeSymbol typeSymbol) + { + var raisesChangeNotifications = typeSymbol.AllInterfaces.Any(contract => contract is + { + Name: "INotifyPropertyChanged", + ContainingNamespace: { Name: "ComponentModel", ContainingNamespace.Name: "System" } + }); + + if (raisesChangeNotifications) + { + _externalContracts.AddNotifyingType(element.Id); + } } private void AnalyzeFieldRelationships(Solution solution, CodeElement fieldElement, IFieldSymbol fieldSymbol) @@ -553,6 +643,8 @@ private void AnalyzePropertyAbstractions(CodeElement propertyElement, IPropertyS { _builder.AddRelationshipWithFallbackToContainingType(propertyElement, overriddenProperty, RelationshipType.Overrides, propertySymbol.GetSymbolLocations(), RelationshipAttribute.None); + + RecordIfExternalContract(propertyElement, overriddenProperty); } } diff --git a/CSharpCodeAnalyst.CodeParser/Parser/GeneratedCode.cs b/CSharpCodeAnalyst.CodeParser/Parser/GeneratedCode.cs new file mode 100644 index 00000000..abb4f738 --- /dev/null +++ b/CSharpCodeAnalyst.CodeParser/Parser/GeneratedCode.cs @@ -0,0 +1,94 @@ +using CSharpCodeAnalyst.CodeGraph.Graph; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; + +namespace CSharpCodeAnalyst.CodeParser.Parser; + +/// +/// Recognizes code a tool wrote. Generated code is always parsed - it holds relationships nothing else +/// does, and dropping it would turn the code it references into dead code (the markup compiler's +/// Connect is the only caller of every XAML event handler, and a CommunityToolkit +/// [ObservableProperty] is the only thing reading the backing field). It is marked instead, so a +/// consumer can leave it out of a result it would only clutter. +/// +/// The rules are Roslyn's own (GeneratedCodeUtilities, what every analyzer uses): a file name +/// from the known list, or an <auto-generated> comment before the first token. +/// +/// +internal static class GeneratedCode +{ + /// + /// File names a tool owns. .g.cs / .g.i.cs come from the XAML markup compiler, + /// .designer.cs from the resx and WinForms designers. + /// + private static readonly string[] GeneratedFileSuffixes = + [ + ".g.cs", ".g.i.cs", ".designer.cs", ".generated.cs", ".assemblyattributes.cs" + ]; + + private const string TemporaryGeneratedFilePrefix = "TemporaryGeneratedFile_"; + + public static bool IsGeneratedFile(SyntaxTree tree) + { + return HasGeneratedFileName(tree.FilePath) || HasAutoGeneratedComment(tree); + } + + /// + /// Whether every declaration of the element sits in a generated file. + /// + /// "Every" is the point. A WPF code-behind class is partial: half of it is + /// MainWindow.xaml.cs, which you wrote, and half is MainWindow.g.cs, which the + /// markup compiler wrote - one element with two source locations. Asking whether *any* + /// declaration is generated would mark your own class; asking about the first one would make + /// the answer depend on the order the files were walked in. Only the generated half's own + /// members (Connect, the x:Name fields) are declared nowhere else, and those are + /// exactly the ones this is meant to catch. + /// + /// + /// An element without any source location - an assembly, a namespace - is never generated. + /// + /// + public static bool IsGeneratedElement(CodeElement element, HashSet generatedFilePaths) + { + return element.SourceLocations.Count > 0 && + element.SourceLocations.All(location => + location.File is not null && generatedFilePaths.Contains(location.File)); + } + + private static bool HasGeneratedFileName(string? filePath) + { + if (string.IsNullOrEmpty(filePath)) + { + return false; + } + + var fileName = Path.GetFileName(filePath); + + return fileName.StartsWith(TemporaryGeneratedFilePrefix, StringComparison.OrdinalIgnoreCase) || + GeneratedFileSuffixes.Any(suffix => fileName.EndsWith(suffix, StringComparison.OrdinalIgnoreCase)); + } + + /// + /// The header convention. The comment has to sit before the first token - otherwise a file merely + /// talking about generated code would count. This is what catches the files the name list does not: + /// the AssemblyInfo under obj, and anything a tool writes under its own name. + /// + private static bool HasAutoGeneratedComment(SyntaxTree tree) + { + foreach (var trivia in tree.GetRoot().GetFirstToken(true).LeadingTrivia) + { + if (!trivia.IsKind(SyntaxKind.SingleLineCommentTrivia) && + !trivia.IsKind(SyntaxKind.MultiLineCommentTrivia)) + { + continue; + } + + if (trivia.ToString().Contains("? _progress; private readonly HashSet _projectFilePaths = []; + + /// The files a tool wrote, collected while walking - see . + private readonly HashSet _generatedFilePaths = new(StringComparer.OrdinalIgnoreCase); + private readonly Dictionary _symbolKeyToElementMap = new(); internal HierarchyAnalyzer(IProgress? progress, ParserConfig config, ParserDiagnostics diagnostics) @@ -49,11 +53,11 @@ internal HierarchyAnalyzer(IProgress? progress, ParserConfig config, Par // Source-generated documents (e.g. CommunityToolkit.Mvvm [ObservableProperty]/[RelayCommand], // [GeneratedRegex], ...) are not part of project.Documents and their syntax trees are not in // the compilation, so they have to be requested explicitly. - IEnumerable generatedDocuments = []; - if (_config.IncludeGeneratedCode) - { - generatedDocuments = await project.GetSourceGeneratedDocumentsAsync(); - } + // Always, and deliberately so: leaving them out does not just hide the generated members, it + // removes the only reference many hand-written ones have. Nothing else reads the backing field + // of an [ObservableProperty], and nothing else calls the method behind a [RelayCommand] - they + // would all turn into dead code. What is generated is marked instead (see MarkGeneratedElements). + var generatedDocuments = await project.GetSourceGeneratedDocumentsAsync(); // Build also a list of all named types in the solution // We need this in phase 2 to resolve relationships @@ -64,6 +68,8 @@ internal HierarchyAnalyzer(IProgress? progress, ParserConfig config, Par await BuildHierarchy(compilation, generatedDocuments); } + MarkGeneratedElements(); + var result = new Artifacts( _allNamedTypesInSolution.AsReadOnly(), _elementIdToSymbolMap.AsReadOnly(), @@ -176,6 +182,27 @@ private bool ShouldAnalyzeProject(Project project) return true; } + /// + /// Roslyn's accessibility onto ours. NotApplicable (namespaces, and anything Roslyn cannot + /// decide) maps to Unknown - the graph must not claim a visibility that does not exist. + /// + private static CodeGraph.Graph.AccessLevel MapAccessLevel( + Microsoft.CodeAnalysis.Accessibility accessibility) + { + return accessibility switch + { + Microsoft.CodeAnalysis.Accessibility.Private => CodeGraph.Graph.AccessLevel.Private, + Microsoft.CodeAnalysis.Accessibility.Protected => CodeGraph.Graph.AccessLevel.Protected, + Microsoft.CodeAnalysis.Accessibility.Internal => CodeGraph.Graph.AccessLevel.Internal, + Microsoft.CodeAnalysis.Accessibility.ProtectedAndInternal => CodeGraph.Graph.AccessLevel + .ProtectedAndInternal, + Microsoft.CodeAnalysis.Accessibility.ProtectedOrInternal => CodeGraph.Graph.AccessLevel + .ProtectedOrInternal, + Microsoft.CodeAnalysis.Accessibility.Public => CodeGraph.Graph.AccessLevel.Public, + _ => CodeGraph.Graph.AccessLevel.Unknown + }; + } + private async Task BuildHierarchy(Compilation compilation, IEnumerable generatedDocuments) { // Assembly has no source location. @@ -190,6 +217,11 @@ private async Task BuildHierarchy(Compilation compilation, IEnumerable continue; } + if (GeneratedCode.IsGeneratedFile(syntaxTree)) + { + _generatedFilePaths.Add(syntaxTree.FilePath); + } + var semanticModel = compilation.GetSemanticModel(syntaxTree); var root = syntaxTree.GetRoot(); @@ -197,10 +229,10 @@ private async Task BuildHierarchy(Compilation compilation, IEnumerable ProcessNodeForHierarchy(root, semanticModel, assemblyElement); } - // Process the source-generated documents (only present when IncludeGeneratedCode is enabled) - // through the same hierarchy walk. The generated members then get their own code element - // instead of being collapsed onto the containing type via the phase-2 fallback. Generated - // members extend existing partial types, so the named types are already collected above. + // Process the source-generated documents through the same hierarchy walk. The generated members + // then get their own code element instead of being collapsed onto the containing type via the + // phase-2 fallback. Generated members extend existing partial types, so the named types are + // already collected above. foreach (var generatedDocument in generatedDocuments) { var semanticModel = await generatedDocument.GetSemanticModelAsync(); @@ -210,10 +242,35 @@ private async Task BuildHierarchy(Compilation compilation, IEnumerable continue; } + // Generated by definition - no file-name or header check needed. + if (!string.IsNullOrEmpty(root.SyntaxTree.FilePath)) + { + _generatedFilePaths.Add(root.SyntaxTree.FilePath); + } + ProcessNodeForHierarchy(root, semanticModel, assemblyElement); } } + /// + /// Flags what a tool wrote. Runs after the whole solution is walked, because the decision needs + /// all declarations of an element: a WPF code-behind class is partial and lives in both a + /// generated and a hand-written file, and only the members that exist nowhere but the generated + /// half are generated (see ). + /// + private void MarkGeneratedElements() + { + if (_generatedFilePaths.Count == 0) + { + return; + } + + foreach (var element in _codeGraph.Nodes.Values) + { + element.IsGenerated = GeneratedCode.IsGeneratedElement(element, _generatedFilePaths); + } + } + private void ProcessNodeForHierarchy(SyntaxNode node, SemanticModel semanticModel, CodeElement parent) { @@ -429,7 +486,10 @@ private CodeElement GetOrCreateCodeElement(ISymbol symbol, CodeElementType eleme var fullName = symbol.BuildSymbolName(); var newId = Guid.NewGuid().ToString(); - var element = new CodeElement(newId, elementType, name, fullName, parent); + var element = new CodeElement(newId, elementType, name, fullName, parent) + { + AccessLevel = MapAccessLevel(symbol.DeclaredAccessibility) + }; UpdateCodeElementLocations(element, location); @@ -478,7 +538,11 @@ private void CreatePropertyAccessorElement(IMethodSymbol? accessor, CodeElement var name = accessor.Name; var fullName = propertyElement.FullName + "." + name; var id = Guid.NewGuid().ToString(); - var accessorElement = new CodeElement(id, CodeElementType.PropertyAccessor, name, fullName, propertyElement); + var accessorElement = new CodeElement(id, CodeElementType.PropertyAccessor, name, fullName, propertyElement) + { + // An accessor may narrow the property ("public int P { get; private set; }"). + AccessLevel = MapAccessLevel(accessor.DeclaredAccessibility) + }; foreach (var accessorLocation in accessor.GetSymbolLocations()) { diff --git a/CSharpCodeAnalyst.CodeParser/Parser/Parser.cs b/CSharpCodeAnalyst.CodeParser/Parser/Parser.cs index 54a31abd..8af7f6fb 100644 --- a/CSharpCodeAnalyst.CodeParser/Parser/Parser.cs +++ b/CSharpCodeAnalyst.CodeParser/Parser/Parser.cs @@ -1,8 +1,10 @@ using System.Diagnostics; using CSharpCodeAnalyst.CodeGraph.Contracts; +using CSharpCodeAnalyst.CodeGraph.Declarations; using CSharpCodeAnalyst.CodeGraph.Graph; using CSharpCodeAnalyst.CodeGraph.Metrics; using CSharpCodeAnalyst.CodeParser.Parser.Config; +using CSharpCodeAnalyst.CodeParser.Xaml; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.MSBuild; @@ -161,6 +163,10 @@ private static MetadataReference[] BuildFrameworkReferences() "System.Linq.Expressions.dll", "System.Collections.dll", "System.Console.dll", + + // INotifyPropertyChanged, ObservableCollection - netstandard.dll only forwards to this + // assembly, so without it those types stay unresolved and miss from AllInterfaces. + "System.ObjectModel.dll", "netstandard.dll" ]; @@ -195,12 +201,20 @@ private async Task ParseSolutionInternal(Solution solution) sw = Stopwatch.StartNew(); // Second Pass: Build Relationships + var externalContracts = new ExternalContractStore(); var phase2 = new RelationshipAnalyzer(progress, config); - await phase2.AnalyzeRelationships(solution, codeGraph, artifacts); + await phase2.AnalyzeRelationships(solution, codeGraph, artifacts, externalContracts); sw.Stop(); Trace.TraceInformation("Analyzing relationships: " + sw.Elapsed); + // Third pass: the XAML references Roslyn cannot see. Runs before the global namespace is inserted + // so the synthetic elements for code-behind-less files are moved along with everything else. + if (config.IncludeXamlReferences) + { + LinkXamlReferences(solution, codeGraph); + } + // Makes the cycle detection easier because I never get to the assembly as shared ancestor // for a nested relationships. InsertGlobalNamespaceIfUsed(codeGraph); @@ -210,10 +224,48 @@ private async Task ParseSolutionInternal(Solution solution) #endif //await File.WriteAllTextAsync("d:\\debug0.txt", codeGraph.ToDebug()); - return new ParseResult(codeGraph, metrics); + return new ParseResult(codeGraph, metrics) { ExternalContracts = externalContracts }; } + /// + /// Adds the references that only exist in XAML. The assembly elements are matched to the Roslyn + /// projects by assembly name; which XAML files a project owns is answered by + /// , because MSBuildWorkspace does not expose the "Page" items. + /// + private void LinkXamlReferences(Solution solution, CodeGraph.Graph.CodeGraph codeGraph) + { + progress?.Report("Reading XAML references ..."); + var sw = Stopwatch.StartNew(); + + var assembliesByName = codeGraph.GetRoots() + .Where(root => root.ElementType == CodeElementType.Assembly) + .ToDictionary(root => root.Name, root => root); + + var projects = new List(); + + // Constructed only here, and only now: it holds an MSBuild ProjectCollection, so the type must not + // be touched before Initializer.InitializeMsBuildLocator has run. + using (var locator = new XamlFileLocator()) + { + foreach (var project in solution.Projects) + { + var directory = Path.GetDirectoryName(project.FilePath); + if (directory is null || !config.IsProjectIncluded(project.Name) || + !assembliesByName.TryGetValue(project.AssemblyName, out var assembly)) + { + continue; + } + + projects.Add(new XamlProject(assembly, directory, locator.Locate(project.FilePath, directory))); + } + } + + var added = XamlGraphLinker.Link(codeGraph, projects); + sw.Stop(); + Trace.TraceInformation($"Reading XAML references: {sw.Elapsed} ({added} relationships)"); + } + /// /// Computes per-member source metrics from the symbol map built in phase 1. /// Only method-like symbols with an actual implementation are measured; abstract/extern/ diff --git a/CSharpCodeAnalyst.CodeParser/Parser/RelationshipAnalyzer.cs b/CSharpCodeAnalyst.CodeParser/Parser/RelationshipAnalyzer.cs index 84140cf8..8af92a2f 100644 --- a/CSharpCodeAnalyst.CodeParser/Parser/RelationshipAnalyzer.cs +++ b/CSharpCodeAnalyst.CodeParser/Parser/RelationshipAnalyzer.cs @@ -1,3 +1,4 @@ +using CSharpCodeAnalyst.CodeGraph.Declarations; using CSharpCodeAnalyst.CodeGraph.Graph; using CSharpCodeAnalyst.CodeParser.Parser.Config; using Microsoft.CodeAnalysis; @@ -34,15 +35,16 @@ public RelationshipAnalyzer(IProgress? progress, ParserConfig config) /// (useful when debugging); the default (-1) lets the scheduler use all available cores. /// public Task AnalyzeRelationships(Solution solution, CodeGraph.Graph.CodeGraph codeGraph, Artifacts artifacts, - int maxDegreeOfParallelism = -1) + ExternalContractStore externalContracts, int maxDegreeOfParallelism = -1) { ArgumentNullException.ThrowIfNull(solution, nameof(solution)); ArgumentNullException.ThrowIfNull(codeGraph, nameof(codeGraph)); ArgumentNullException.ThrowIfNull(artifacts, nameof(artifacts)); + ArgumentNullException.ThrowIfNull(externalContracts, nameof(externalContracts)); var builder = new RelationshipBuilder(codeGraph, artifacts, _config); var bodyAnalyzer = new SyntaxNodeAnalyzer(builder, _config); - var declarationAnalyzer = new DeclarationAnalyzer(builder, bodyAnalyzer, artifacts, _config); + var declarationAnalyzer = new DeclarationAnalyzer(builder, bodyAnalyzer, artifacts, _config, externalContracts); var numberOfCodeElements = codeGraph.Nodes.Count; _processedCodeElements = 0; diff --git a/CSharpCodeAnalyst.CodeParser/Xaml/XamlFileLocator.cs b/CSharpCodeAnalyst.CodeParser/Xaml/XamlFileLocator.cs new file mode 100644 index 00000000..e6166525 --- /dev/null +++ b/CSharpCodeAnalyst.CodeParser/Xaml/XamlFileLocator.cs @@ -0,0 +1,139 @@ +using System.Diagnostics; +using Microsoft.Build.Evaluation; +using Microsoft.Build.Locator; + +namespace CSharpCodeAnalyst.CodeParser.Xaml; + +/// +/// Answers which XAML files belong to a project. +/// +/// Roslyn cannot tell us: a Microsoft.CodeAnalysis.Project exposes Documents, +/// AdditionalDocuments and AnalyzerConfigDocuments, and a Page is none of them. The project +/// file is therefore evaluated a second time, with the MSBuild engine +/// Initializer.InitializeMsBuildLocator has already put in place. That gives the item list +/// the build itself works with - in particular a file taken out again by +/// <Page Remove="..." /> appears in no item group at all and is correctly gone. +/// +/// +/// Scanning the directory - what this did before - reads whatever happens to lie there: a file +/// excluded from the project, a leftover from another branch, or the XAML of a project nested +/// inside this one. It remains the fallback for a project file that cannot be evaluated. +/// +/// +/// A linked file (<Page Include="..\Shared\Foo.xaml">) comes along for free, because +/// the item carries its real path. That is a side effect, not the goal - the directory scan misses +/// those and always did. +/// +/// +/// The evaluation costs a few hundred milliseconds per project (the first one more, it warms up the +/// engine). Every project shares one so the SDK imports are +/// evaluated once, which is why this is an instance and not a static helper. +/// +/// +public sealed class XamlFileLocator : IDisposable +{ + /// + /// The item types a XAML file can legitimately have. Everything else is either not XAML we could + /// read or not ours: the SDK contributes twenty PropertyPageSchema items pointing into the + /// dotnet installation, which a plain "all evaluated items ending in .xaml" would pick up. + /// + private static readonly HashSet XamlItemTypes = new(StringComparer.OrdinalIgnoreCase) + { + // Compiled to BAML by the WPF markup compiler. + "ApplicationDefinition", "Page", + + // Embedded or copied and loaded at runtime (loose XAML, themes). + "Resource", "Content", + + // What the SDK default glob puts a stray XAML file into when the project does not compile it. + "None" + }; + + /// + /// The , created on first use and typed as on + /// purpose: the field's type appears in , and touching an MSBuild type there + /// would load the assembly even for a run that never evaluates a project - an in-memory parse has + /// no MSBuild at all. + /// + private object? _collection; + + public void Dispose() + { + (_collection as IDisposable)?.Dispose(); + } + + /// + /// The project file. Null for a project Roslyn produced without one, which leaves only the scan. + /// + /// The directory used by the fallback scan. + public IReadOnlyList Locate(string? projectFilePath, string projectDirectory) + { + // Two ways to have nothing to evaluate, both of them normal rather than a defect: an in-memory + // parse (ParseSourceAsync) builds its project around the synthetic path "InMemory.csproj" and + // never registers a locator, and without a registered locator there is no MSBuild at all. + if (projectFilePath is not null && File.Exists(projectFilePath) && MSBuildLocator.IsRegistered) + { + try + { + return FromProjectFile(projectFilePath); + } + catch (Exception exception) + { + // A project we cannot evaluate must not cost us its references - and it must not break the + // parse run either. Broad on purpose: MSBuild throws its own exception type for an invalid + // project, IO exceptions for a file that moved, the SDK resolvers can fail on their own, + // and preparing FromProjectFile is where a missing MSBuild assembly would surface. + Trace.TraceWarning( + $"XAML: cannot evaluate '{projectFilePath}', falling back to a directory scan. {exception.Message}"); + } + } + + return EnumerateDirectory(projectDirectory); + } + + /// + /// An empty result is an answer, not a failure: a project without XAML items has no XAML, and + /// falling back to the scan here would bring the excluded files straight back in. + /// + private IReadOnlyList FromProjectFile(string projectFilePath) + { + // Evaluated without global properties. The XAML item groups are not written per configuration in + // any project we have seen, and guessing the configuration Roslyn used would be worse than not + // setting one. + var collection = (ProjectCollection)(_collection ??= new ProjectCollection()); + var project = collection.LoadProject(projectFilePath); + + return project.AllEvaluatedItems + .Where(item => XamlItemTypes.Contains(item.ItemType)) + .Select(item => item.GetMetadataValue("FullPath")) + .Where(IsXamlFile) + .Distinct(StringComparer.OrdinalIgnoreCase) + .ToList(); + } + + /// + /// The fallback: every XAML file below the directory. The output directories are skipped - the + /// markup compiler copies XAML into obj, and everything found there is a duplicate. + /// + public static IReadOnlyList EnumerateDirectory(string directory) + { + if (!Directory.Exists(directory)) + { + return []; + } + + return Directory.EnumerateFiles(directory, "*.xaml", SearchOption.AllDirectories) + .Where(file => !file.Contains($"{Path.DirectorySeparatorChar}obj{Path.DirectorySeparatorChar}") && + !file.Contains($"{Path.DirectorySeparatorChar}bin{Path.DirectorySeparatorChar}")) + .ToList(); + } + + /// + /// An item type from the list above can hold anything (a Content item is usually not XAML), + /// and an item can name a file that is not on disk. + /// + private static bool IsXamlFile(string path) + { + return path.EndsWith(".xaml", StringComparison.OrdinalIgnoreCase) && File.Exists(path); + } +} diff --git a/CSharpCodeAnalyst.CodeParser/Xaml/XamlGraphLinker.cs b/CSharpCodeAnalyst.CodeParser/Xaml/XamlGraphLinker.cs new file mode 100644 index 00000000..43e1f6e6 --- /dev/null +++ b/CSharpCodeAnalyst.CodeParser/Xaml/XamlGraphLinker.cs @@ -0,0 +1,270 @@ +using CSharpCodeAnalyst.CodeGraph.Graph; + +namespace CSharpCodeAnalyst.CodeParser.Xaml; + +/// +/// One analyzed project: its assembly element in the graph, its directory and the XAML files belonging +/// to it. Which files those are is an MSBuild question and is answered by +/// - the directory is only needed to name the synthetic element of a file without code-behind. +/// +public sealed record XamlProject(CodeElement Assembly, string Directory, IReadOnlyList Files); + +/// +/// Turns the references finds into real relationships in the code +/// graph, so a type that is only ever instantiated from XAML no longer looks unused. +/// +/// The source of such a relationship is the code-behind class named by x:Class. A resource +/// dictionary has no code-behind, so a synthetic class named after the file takes its place - +/// the same device the parser already uses for top-level statements ("GlobalStatements"). Those +/// synthetic elements have no incoming references of their own (nothing resolves the +/// MergedDictionaries URIs), so they show up in a dead code analysis. That is a known and +/// accepted cost; there were 14 of them in this repository against 1050 findings. +/// +/// +/// Resolution is by exact name, never by guessing: the xmlns gives the CLR namespace and optionally +/// the assembly. Without ;assembly= XAML means the assembly the file is compiled into, which +/// is what is tried first; a unique match elsewhere is accepted as a fallback. +/// +/// +public static class XamlGraphLinker +{ + /// The element name the parser gives a constructor (it comes straight from the symbol). + private const string ConstructorName = ".ctor"; + + public static int Link(CodeGraph.Graph.CodeGraph graph, IReadOnlyList projects) + { + ArgumentNullException.ThrowIfNull(graph); + ArgumentNullException.ThrowIfNull(projects); + + var typesByAssembly = BuildTypeLookup(graph); + var added = 0; + + foreach (var project in projects) + { + foreach (var file in project.Files) + { + added += LinkFile(graph, project, file, typesByAssembly); + } + } + + return added; + } + + private static int LinkFile(CodeGraph.Graph.CodeGraph graph, XamlProject project, string file, + Dictionary> typesByAssembly) + { + XamlFileReferences references; + try + { + references = XamlReferenceExtractor.Extract(File.ReadAllText(file)); + } + catch (IOException) + { + // An unreadable file must not break the parse run. + return 0; + } + + if (references.References.Count == 0) + { + return 0; + } + + var source = ResolveSource(graph, project, file, references, typesByAssembly); + var added = 0; + + foreach (var reference in references.References) + { + foreach (var target in ResolveTargets(project, reference, typesByAssembly)) + { + if (target.Id == source.Id) + { + continue; + } + + if (AddReference(source, target, file, reference)) + { + added++; + } + } + } + + return added; + } + + /// + /// Adds the relationship, or merges the location into the existing one. The relationship set is + /// keyed by (source, target, type), so a plain Add would silently drop the new source location. + /// + private static bool AddReference(CodeElement source, CodeElement target, string file, XamlReference reference) + { + var location = new SourceLocation(file, reference.Line, reference.Column); + var existing = source.Relationships.FirstOrDefault( + r => r.TargetId == target.Id && r.Type == RelationshipType.Uses); + + if (existing is not null) + { + if (!existing.SourceLocations.Contains(location)) + { + existing.SourceLocations.Add(location); + } + + existing.SetAttribute(RelationshipAttribute.IsXamlReference); + return false; + } + + var relationship = new Relationship(source.Id, target.Id, RelationshipType.Uses, + RelationshipAttribute.IsXamlReference); + relationship.SourceLocations.Add(location); + source.Relationships.Add(relationship); + return true; + } + + private static CodeElement ResolveSource(CodeGraph.Graph.CodeGraph graph, XamlProject project, string file, + XamlFileReferences references, Dictionary> typesByAssembly) + { + if (references.CodeBehindClass is not null && + typesByAssembly.TryGetValue(project.Assembly.Name, out var types) && + types.TryGetValue(references.CodeBehindClass, out var codeBehind)) + { + return codeBehind; + } + + return GetOrCreateSyntheticElement(graph, project, file); + } + + /// + /// The stand-in for a XAML file that has no code-behind class. Named after the path relative to the + /// project so two files with the same name stay distinguishable. A linked file lies outside the + /// project directory, where a relative path would only produce a row of dots - its file name has to + /// do. + /// + private static CodeElement GetOrCreateSyntheticElement(CodeGraph.Graph.CodeGraph graph, XamlProject project, + string file) + { + var relativePath = Path.GetRelativePath(project.Directory, file); + if (relativePath.StartsWith("..", StringComparison.Ordinal) || Path.IsPathRooted(relativePath)) + { + relativePath = Path.GetFileName(file); + } + + var name = Path.ChangeExtension(relativePath, null) + .Replace(Path.DirectorySeparatorChar, '.') + .Replace(Path.AltDirectorySeparatorChar, '.'); + + var fullName = project.Assembly.FullName + "." + name; + + var existing = project.Assembly.Children.FirstOrDefault(c => c.FullName == fullName); + if (existing is not null) + { + return existing; + } + + var element = new CodeElement(Guid.NewGuid().ToString(), CodeElementType.Class, name, fullName, + project.Assembly); + element.SourceLocations.Add(new SourceLocation(file, 1, 1)); + + project.Assembly.Children.Add(element); + graph.Nodes[element.Id] = element; + return element; + } + + private static IEnumerable ResolveTargets(XamlProject project, XamlReference reference, + Dictionary> typesByAssembly) + { + var type = ResolveType(project, reference, typesByAssembly); + if (type is null) + { + yield break; + } + + if (reference.MemberName is not null) + { + // {x:Static Type.Member} - prefer the member, fall back to the type when it has no element + // (e.g. an enum value or a member the parser did not model). + yield return type.Children.FirstOrDefault(c => c.Name == reference.MemberName) ?? type; + yield break; + } + + yield return type; + + if (!reference.IsInstantiation) + { + yield break; + } + + // An object element runs the constructor. Without this edge the constructor has no incoming + // reference at all, and everything only it calls dies with it in the cascade - the body of a + // XAML-instantiated control lives almost entirely below its constructor. + // Overloads share the element name, so all of them are linked: XAML picks the parameterless one, + // but the graph cannot tell them apart, and an edge too many is far cheaper here than a missing + // one. + foreach (var constructor in type.Children.Where(IsConstructor)) + { + yield return constructor; + } + } + + private static bool IsConstructor(CodeElement element) + { + return element is { ElementType: CodeElementType.Method, Name: ConstructorName }; + } + + private static CodeElement? ResolveType(XamlProject project, XamlReference reference, + Dictionary> typesByAssembly) + { + // An explicit ";assembly=" wins; without it XAML means the assembly the file is compiled into. + var assemblyName = reference.AssemblyName ?? project.Assembly.Name; + if (typesByAssembly.TryGetValue(assemblyName, out var types) && + types.TryGetValue(reference.TypeFullName, out var declared)) + { + return declared; + } + + // Fallback: a unique match anywhere. Ambiguous names are dropped rather than guessed. + var matches = typesByAssembly.Values + .Select(candidates => candidates.GetValueOrDefault(reference.TypeFullName)) + .Where(candidate => candidate is not null) + .Take(2) + .ToList(); + + return matches.Count == 1 ? matches[0] : null; + } + + /// + /// Maps assembly name -> CLR full name ("Namespace.Type") -> type element. The assembly node and + /// the synthetic global namespace are not part of a CLR name and are skipped. + /// + private static Dictionary> BuildTypeLookup( + CodeGraph.Graph.CodeGraph graph) + { + var lookup = new Dictionary>(); + + foreach (var element in graph.Nodes.Values) + { + if (!element.IsType() || element.IsExternal) + { + continue; + } + + var path = element.GetPathToRoot(true); + if (path.Count < 2 || path[0].ElementType != CodeElementType.Assembly) + { + continue; + } + + var segments = path.Skip(1) + .Where(p => p.ElementType != CodeElementType.Namespace || + p.Name != CodeElement.GlobalNamespaceName) + .Select(p => p.Name); + + var types = lookup.TryGetValue(path[0].Name, out var existing) ? existing : lookup[path[0].Name] = []; + + // A name collision would mean two types with the same full name in one assembly, which the + // compiler would already have rejected. + types[string.Join(".", segments)] = element; + } + + return lookup; + } + +} diff --git a/CSharpCodeAnalyst.CodeParser/Xaml/XamlReferenceExtractor.cs b/CSharpCodeAnalyst.CodeParser/Xaml/XamlReferenceExtractor.cs new file mode 100644 index 00000000..b1eda486 --- /dev/null +++ b/CSharpCodeAnalyst.CodeParser/Xaml/XamlReferenceExtractor.cs @@ -0,0 +1,211 @@ +using System.Text.RegularExpressions; +using System.Xml; +using System.Xml.Linq; + +namespace CSharpCodeAnalyst.CodeParser.Xaml; + +/// +/// A single reference to CLR code found in a XAML file. is null when the +/// whole type is referenced (an element tag, {x:Type}), and set for {x:Static}. +/// comes from the ;assembly= part of the xmlns and is null when the +/// xmlns omits it - which means the type lives in the same assembly as the XAML file. +/// +public sealed record XamlReference( + string NamespaceName, + string TypeName, + string? MemberName, + string? AssemblyName, + int Line, + int Column) +{ + /// + /// True for an object element (<local:MyControl/>) - XAML creates an instance there, + /// so the constructor runs. False for everything that only names a type: property element syntax + /// (<local:MyControl.Items>), an attached property and {x:Type}. + /// + public bool IsInstantiation { get; init; } + + public string TypeFullName => $"{NamespaceName}.{TypeName}"; +} + +/// +/// Everything one XAML file contributes: the code-behind class it belongs to (from x:Class, null +/// for a resource dictionary) and the CLR references it makes. +/// +public sealed class XamlFileReferences +{ + public string? CodeBehindClass { get; init; } + public IReadOnlyList References { get; init; } = []; +} + +/// +/// Reads the CLR references out of a XAML file - the ones the markup compiler does *not* turn into C#. +/// +/// The WPF markup compiler generates a partial class per XAML file that contains the event handler +/// wiring and a field per x:Name, so those references are already visible to Roslyn. What +/// never reaches C# is everything declarative: it is compiled into BAML and resolved by reflection +/// at runtime. Three of those constructs carry a fully qualified CLR name and can therefore be +/// resolved exactly, which is what this extractor collects: +/// +/// +/// element tags - <local:MyControl/>, including property element syntax +/// {x:Static local:Texts.Caption} +/// {x:Type local:Foo} +/// +/// +/// {Binding Path} is deliberately NOT collected. Without evaluating the DataContext it is a +/// bare member name, and matching that by name across the whole codebase would suppress far more +/// than it explains. +/// +/// +/// Prefixes are resolved through the XML namespace declarations, so a clr-namespace xmlns is +/// mapped exactly - there is no name guessing anywhere in here. +/// +/// +public static class XamlReferenceExtractor +{ + private const string ClrNamespacePrefix = "clr-namespace:"; + + /// + /// Typically named x: + /// + private const string XamlNamespace = "http://schemas.microsoft.com/winfx/2006/xaml"; + + /// + /// Matches "{prefix:Static target:Type.Member}" and "{prefix:Type target:Type}", also when nested + /// inside another markup extension. Both prefixes are resolved against the element, never assumed. + /// + private static readonly Regex MarkupExtension = new( + @"\{\s*(?\w+)\s*:\s*(?Static|Type)\s+(?\w+)\s*:\s*(?[\w.]+)", + RegexOptions.Compiled); + + public static XamlFileReferences Extract(string xaml) + { + ArgumentNullException.ThrowIfNull(xaml); + + XDocument document; + try + { + document = XDocument.Parse(xaml, LoadOptions.SetLineInfo); + } + catch (XmlException) + { + // A malformed or unsupported file contributes nothing. It must never break the parse run. + return new XamlFileReferences(); + } + + var references = new List(); + + foreach (var element in document.Descendants()) + { + CollectElementTag(element, references); + + foreach (var attribute in element.Attributes()) + { + CollectAttachedProperty(attribute, references); + CollectMarkupExtensions(element, attribute, references); + } + } + + return new XamlFileReferences + { + CodeBehindClass = document.Root?.Attribute(XName.Get("Class", XamlNamespace))?.Value, + References = references + }; + } + + /// + /// The element tag itself: <local:MyControl/>. Property element syntax puts the property + /// behind a dot (<local:MyControl.Items>), so only the part in front of it is the type. + /// + private static void CollectElementTag(XElement element, List references) + { + // Only a tag without a dot creates an object; with one it is property element syntax. + // This is not necessary redundant: ... + var isInstantiation = !element.Name.LocalName.Contains('.'); + Add(element.Name.NamespaceName, element.Name.LocalName, element, references, + isInstantiation: isInstantiation); + } + + /// An attached property written as local:MyPanel.Dock="..." references MyPanel. + private static void CollectAttachedProperty(XAttribute attribute, List references) + { + Add(attribute.Name.NamespaceName, attribute.Name.LocalName, attribute, references); + } + + private static void CollectMarkupExtensions(XElement element, XAttribute attribute, + List references) + { + foreach (Match match in MarkupExtension.Matches(attribute.Value)) + { + // "x" is only a convention - verify the prefix really maps to the XAML language namespace. + var xamlNamespace = element.GetNamespaceOfPrefix(match.Groups["xamlPrefix"].Value); + if (xamlNamespace?.NamespaceName != XamlNamespace) + { + continue; + } + + var targetNamespace = element.GetNamespaceOfPrefix(match.Groups["prefix"].Value); + if (targetNamespace is null) + { + continue; + } + + var path = match.Groups["path"].Value; + if (match.Groups["kind"].Value == "Type") + { + Add(targetNamespace.NamespaceName, path, attribute, references); + continue; + } + + // {x:Static Type.Member} - the last segment is the member. + var separator = path.LastIndexOf('.'); + if (separator <= 0 || separator == path.Length - 1) + { + continue; + } + + Add(targetNamespace.NamespaceName, path[..separator], attribute, references, + path[(separator + 1)..]); + } + } + + private static void Add(string namespaceName, string localName, IXmlLineInfo position, + List references, string? memberName = null, bool isInstantiation = false) + { + if (!namespaceName.StartsWith(ClrNamespacePrefix, StringComparison.Ordinal)) + { + // A framework namespace (presentation, xaml, ...) - nothing of ours is referenced. + return; + } + + // "clr-namespace:Some.Namespace;assembly=Some.Assembly" - the assembly part is optional and + // absent exactly when the type lives in the same assembly as the XAML file. + var declaration = namespaceName[ClrNamespacePrefix.Length..]; + var semicolon = declaration.IndexOf(';'); + var clrNamespace = semicolon < 0 ? declaration : declaration[..semicolon]; + + string? assemblyName = null; + if (semicolon >= 0) + { + const string assemblyKey = "assembly="; + var assemblyPart = declaration[(semicolon + 1)..].Trim(); + if (assemblyPart.StartsWith(assemblyKey, StringComparison.Ordinal)) + { + assemblyName = assemblyPart[assemblyKey.Length..].Trim(); + } + } + + // Property element syntax: names the type in front of the dot. + var dot = localName.IndexOf('.'); + var typeName = dot < 0 ? localName : localName[..dot]; + + if (clrNamespace.Length == 0 || typeName.Length == 0) + { + return; + } + + references.Add(new XamlReference(clrNamespace, typeName, memberName, assemblyName, + position.LineNumber, position.LinePosition) { IsInstantiation = isInstantiation }); + } +} diff --git a/CSharpCodeAnalyst/App.xaml.cs b/CSharpCodeAnalyst/App.xaml.cs index 91236643..c772ad10 100644 --- a/CSharpCodeAnalyst/App.xaml.cs +++ b/CSharpCodeAnalyst/App.xaml.cs @@ -103,8 +103,12 @@ private void StartUi() // project load, read by the Method Complexity analyzer. var metricStore = new CodeGraph.Metrics.MetricStore(); + // Same shape: which members implement a contract from outside the analyzed code. Read by the + // Dead Code analyzer, which would otherwise report every framework override as unused. + var externalContractStore = new CodeGraph.Declarations.ExternalContractStore(); + var analyzerManager = new AnalyzerManager(); - analyzerManager.LoadAnalyzers(messaging, uiNotification, metricStore); + analyzerManager.LoadAnalyzers(messaging, uiNotification, metricStore, externalContractStore); var explorer = new CodeGraphExplorer(); var mainWindow = new MainWindow(); @@ -123,7 +127,7 @@ private void StartUi() var projectStorage = new JsonProjectStorage(); var projectService = new ProjectService(projectStorage, uiNotification, userSettings); - var viewModel = new MainViewModel(messaging, applicationSettings, userSettings, analyzerManager, refactoringService, projectService, metricStore); + var viewModel = new MainViewModel(messaging, applicationSettings, userSettings, analyzerManager, refactoringService, projectService, metricStore, externalContractStore); var graphViewModel = new GraphViewModel(graphViewState, explorer, messaging, applicationSettings, refactoringService); var treeViewModel = new TreeViewModel(messaging, refactoringService); var searchViewModel = new AdvancedSearchViewModel(messaging, refactoringService); diff --git a/CSharpCodeAnalyst/CommandLine/ConsoleValidationCommand.cs b/CSharpCodeAnalyst/CommandLine/ConsoleValidationCommand.cs index c929640c..5e0b9213 100644 --- a/CSharpCodeAnalyst/CommandLine/ConsoleValidationCommand.cs +++ b/CSharpCodeAnalyst/CommandLine/ConsoleValidationCommand.cs @@ -1,4 +1,4 @@ -using CSharpCodeAnalyst.CodeGraph.Contracts; +using CSharpCodeAnalyst.CodeGraph.Contracts; using System.Diagnostics; using System.IO; using System.Text; @@ -93,7 +93,7 @@ private static RuleAnalysisResult RunAnalysis(string rulesFilePath, CodeGraph.Gr { var filter = new ProjectExclusionRegExCollection(); filter.Initialize(settings.DefaultProjectExcludeFilter); - var parser = new Parser(new ParserConfig(filter, settings.IncludeExternalCode, settings.IncludeGeneratedCode, settings.SplitPropertyAccessors)); + var parser = new Parser(new ParserConfig(filter, settings.IncludeExternalCode, settings.SplitPropertyAccessors)); var parseResult = await parser.ParseAsync(solutionPath).ConfigureAwait(false); var failures = parser.Diagnostics.FormatFailures(); diff --git a/CSharpCodeAnalyst/Configuration/AppSettings.cs b/CSharpCodeAnalyst/Configuration/AppSettings.cs index cee208b7..452f53c5 100644 --- a/CSharpCodeAnalyst/Configuration/AppSettings.cs +++ b/CSharpCodeAnalyst/Configuration/AppSettings.cs @@ -1,4 +1,4 @@ -using System.IO; +using System.IO; using System.Text.Json; namespace CSharpCodeAnalyst.Configuration; @@ -17,8 +17,6 @@ public string DefaultProjectExcludeFilter public bool IncludeExternalCode { get; set; } - public bool IncludeGeneratedCode { get; set; } - public bool SplitPropertyAccessors { get; set; } = true; public bool WarnIfFiltersActive { get; set; } = true; @@ -58,7 +56,6 @@ public AppSettings Clone() DefaultProjectExcludeFilter = this.DefaultProjectExcludeFilter, AutomaticallyAddContainingType = this.AutomaticallyAddContainingType, IncludeExternalCode = this.IncludeExternalCode, - IncludeGeneratedCode = this.IncludeGeneratedCode, SplitPropertyAccessors = this.SplitPropertyAccessors, WarnIfFiltersActive = this.WarnIfFiltersActive, ShowOverviewOnImport = this.ShowOverviewOnImport diff --git a/CSharpCodeAnalyst/Configuration/SettingsDialog.xaml b/CSharpCodeAnalyst/Configuration/SettingsDialog.xaml index 21fa0ebd..3686613f 100644 --- a/CSharpCodeAnalyst/Configuration/SettingsDialog.xaml +++ b/CSharpCodeAnalyst/Configuration/SettingsDialog.xaml @@ -1,4 +1,4 @@ - - - RaiseAnalyzerDataChanged(); _analyzers.Add(analyzer.Id, analyzer); + analyzer = new DeadCode.Analyzer(messaging, userNotification, externalContractStore); + analyzer.DataChanged += (_, _) => RaiseAnalyzerDataChanged(); + _analyzers.Add(analyzer.Id, analyzer); + } diff --git a/CSharpCodeAnalyst/Features/Import/Importer.cs b/CSharpCodeAnalyst/Features/Import/Importer.cs index 58a47339..51f3888a 100644 --- a/CSharpCodeAnalyst/Features/Import/Importer.cs +++ b/CSharpCodeAnalyst/Features/Import/Importer.cs @@ -1,4 +1,4 @@ -using CSharpCodeAnalyst.AnalyzerSdk.Notifications; +using CSharpCodeAnalyst.AnalyzerSdk.Notifications; using CSharpCodeAnalyst.CodeGraph.Contracts; using CSharpCodeAnalyst.CodeParser.Parser; using CSharpCodeAnalyst.CodeParser.Parser.Config; @@ -45,7 +45,7 @@ public Importer(IUserNotification ui, IProgress busy) } public async Task> ImportSolutionAsync(ProjectExclusionRegExCollection filters, bool includeExternalCode, - bool includeGeneratedCode, bool splitPropertyAccessors) + bool splitPropertyAccessors) { var fileName = TryGetImportSolutionPath(); if (string.IsNullOrEmpty(fileName)) @@ -56,7 +56,7 @@ public async Task> ImportSolutionAsync(ProjectExclusionRegEx var result = await ExecuteGuardedImportAsync( Strings.Load_Message_Default, async () => (ParseResult?)await Task.Run(() => - ImportSolutionFuncAsync(fileName, filters, includeExternalCode, includeGeneratedCode, splitPropertyAccessors))); + ImportSolutionFuncAsync(fileName, filters, includeExternalCode, splitPropertyAccessors))); if (_parserDiagnostics is { HasDiagnostics: true }) { @@ -87,9 +87,9 @@ public async Task> RunImporterAsync(IImporter importer) } private async Task ImportSolutionFuncAsync(string solutionPath, ProjectExclusionRegExCollection filters, - bool includeExternalCode, bool includeGeneratedCode, bool splitPropertyAccessors) + bool includeExternalCode, bool splitPropertyAccessors) { - var parser = new Parser(new ParserConfig(filters, includeExternalCode, includeGeneratedCode, splitPropertyAccessors), _progress); + var parser = new Parser(new ParserConfig(filters, includeExternalCode, splitPropertyAccessors), _progress); _parserDiagnostics = null; var parseResult = await parser.ParseAsync(solutionPath).ConfigureAwait(true); diff --git a/CSharpCodeAnalyst/Features/Tree/TreeViewModel.cs b/CSharpCodeAnalyst/Features/Tree/TreeViewModel.cs index 7ebb4fc2..adca4fc0 100644 --- a/CSharpCodeAnalyst/Features/Tree/TreeViewModel.cs +++ b/CSharpCodeAnalyst/Features/Tree/TreeViewModel.cs @@ -398,7 +398,10 @@ public void ExecuteSearch() } else { - var expr = SearchExpressionFactory.CreateSearchExpression(SearchText, SearchExpressionFactory.TextSearchField.Name); + // No negation here: SearchAndExpandNodes expands and highlights every ancestor of a match, so + // an excluding search would match nearly everything and unfold the whole tree at once. + var expr = SearchExpressionFactory.CreateSearchExpression(SearchText, + SearchExpressionFactory.TextSearchField.Name, false); SearchAndExpandNodes(TreeItems, expr); } } diff --git a/CSharpCodeAnalyst/MainViewModel.cs b/CSharpCodeAnalyst/MainViewModel.cs index b71ce85e..cb2f13c5 100644 --- a/CSharpCodeAnalyst/MainViewModel.cs +++ b/CSharpCodeAnalyst/MainViewModel.cs @@ -1,4 +1,4 @@ -using CSharpCodeAnalyst.CodeGraph.Contracts; +using CSharpCodeAnalyst.CodeGraph.Contracts; using System.Collections.ObjectModel; using System.ComponentModel; using System.Diagnostics; @@ -15,6 +15,7 @@ using CSharpCodeAnalyst.CodeGraph.Algorithms.Cycles; using CSharpCodeAnalyst.CodeGraph.Algorithms.Partitioning; using CSharpCodeAnalyst.CodeGraph.Graph; +using CSharpCodeAnalyst.CodeGraph.Declarations; using CSharpCodeAnalyst.CodeGraph.Metrics; using CSharpCodeAnalyst.CodeParser.Parser; using CSharpCodeAnalyst.CodeParser.Parser.Config; @@ -65,6 +66,7 @@ internal sealed class MainViewModel : INotifyPropertyChanged private readonly ImporterManager _importerManager = new(); private readonly MessageBus _messaging; + private readonly ExternalContractStore _externalContractStore; private readonly MetricStore _metricStore; private readonly ProjectExclusionRegExCollection _projectExclusionFilters; @@ -90,7 +92,7 @@ internal sealed class MainViewModel : INotifyPropertyChanged internal MainViewModel(MessageBus messaging, AppSettings settings, UserPreferences userSettings, AnalyzerManager analyzerManager, RefactoringService refactoringService, IProjectService projectService, - MetricStore metricStore) + MetricStore metricStore, ExternalContractStore externalContractStore) { // Initialize settings _applicationSettings = settings; @@ -98,6 +100,7 @@ internal MainViewModel(MessageBus messaging, AppSettings settings, UserPreferenc _analyzerManager = analyzerManager; _refactoringService = refactoringService; _metricStore = metricStore; + _externalContractStore = externalContractStore; analyzerManager.AnalyzerDataChanged += OnAnalyzerDataChanged; @@ -977,6 +980,7 @@ private void LoadCodeGraph(CodeGraph.Graph.CodeGraph codeGraph) Cycles = null; DynamicTabs.Clear(); _metricStore.Clear(); + _externalContractStore.Clear(); InfoPanelViewModel?.ClearQuickInfo(); UpdateStatistics(codeGraph); @@ -999,7 +1003,7 @@ private async void OnImportSolution() { AskUserToSaveProject(); - var result = await _importer.ImportSolutionAsync(_projectExclusionFilters, _applicationSettings.IncludeExternalCode, _applicationSettings.IncludeGeneratedCode, + var result = await _importer.ImportSolutionAsync(_projectExclusionFilters, _applicationSettings.IncludeExternalCode, _applicationSettings.SplitPropertyAccessors); if (result.IsCanceled) @@ -1026,6 +1030,10 @@ private void CompleteImport(ParseResult parseResult) // Carry the freshly collected source metrics into the shared store (empty if the option was off). _metricStore.LoadFrom(parseResult.Metrics.Metrics); + // Same for the external contracts (empty for every importer except the C# parser). + _externalContractStore.LoadFrom(parseResult.ExternalContracts.Contracts, + parseResult.ExternalContracts.NotifyingTypes); + // Give an immediate overview of the freshly imported solution: the whole graph, every // container collapsed, so the user starts from a map instead of an empty canvas. Only on // import - loading a saved project restores the user's own view instead. Opt-out via setting. @@ -1247,6 +1255,7 @@ private ProjectData CollectProjectData() projectData.Settings.ExclusionFilter = _projectExclusionFilters.ToString(); projectData.AnalyzerData = _analyzerManager.CollectAnalyzerData(); projectData.SetMetrics(_metricStore); + projectData.SetExternalContracts(_externalContractStore); return projectData; } @@ -1284,6 +1293,7 @@ private void RestoreProjectData(ProjectData projectData) // Restore the source metrics (LoadCodeGraph cleared the shared store). // Singleton share with analyzer! _metricStore.LoadFrom(projectData.GetMetrics()); + _externalContractStore.LoadFrom(projectData.GetExternalContracts(), projectData.GetNotifyingTypes()); // Restore analyzer data _analyzerManager.RestoreAnalyzerData(projectData.AnalyzerData); diff --git a/CSharpCodeAnalyst/Persistence/Dto/ProjectData.cs b/CSharpCodeAnalyst/Persistence/Dto/ProjectData.cs index 09ca2be0..70e81012 100644 --- a/CSharpCodeAnalyst/Persistence/Dto/ProjectData.cs +++ b/CSharpCodeAnalyst/Persistence/Dto/ProjectData.cs @@ -1,3 +1,4 @@ +using CSharpCodeAnalyst.CodeGraph.Declarations; using CSharpCodeAnalyst.CodeGraph.Graph; using CSharpCodeAnalyst.CodeGraph.Metrics; using CSharpCodeAnalyst.Features.Gallery; @@ -26,6 +27,21 @@ public class ProjectData /// public List MemberMetrics { get; set; } = []; + /// + /// Which members implement a contract from outside the analyzed code, keyed by element id. + /// Empty for every graph producer except the C# parser. An older project file simply has none, + /// and those members show up as unreferenced again until the solution is parsed anew. + /// + public Dictionary ExternalContracts { get; set; } = new(); + + /// + /// The element ids of the types that raise change notifications (INotifyPropertyChanged anywhere + /// in the interface set). Complements : the member-level contract + /// cannot see a view model whose base class lives outside the analyzed code. An older project + /// file simply has none, and the binding rule is off until the solution is parsed anew. + /// + public List NotifyingTypes { get; set; } = []; + /// /// Gallery is already serializable. /// @@ -62,6 +78,22 @@ public Dictionary GetMetrics() }); } + public void SetExternalContracts(ExternalContractStore store) + { + ExternalContracts = store.Contracts.ToDictionary(kvp => kvp.Key, kvp => kvp.Value); + NotifyingTypes = store.NotifyingTypes.ToList(); + } + + public IReadOnlyDictionary GetExternalContracts() + { + return ExternalContracts; + } + + public IReadOnlyCollection GetNotifyingTypes() + { + return NotifyingTypes; + } + /// /// Flatten the recursive structures. /// @@ -70,7 +102,7 @@ public void SetCodeGraph(CodeGraph.Graph.CodeGraph codeGraph) CodeElements = codeGraph.Nodes.Values .Select(n => new SerializableCodeElement(n.Id, n.Name, n.FullName, n.ElementType, n.SourceLocations, n.Attributes, - n.IsExternal)) + n.IsExternal, n.AccessLevel, n.IsGenerated)) .ToList(); // We iterate over children, so we expect to have a parent @@ -98,7 +130,9 @@ public CodeGraph.Graph.CodeGraph GetCodeGraph() { SourceLocations = se.SourceLocations, Attributes = se.Attributes, - IsExternal = se.IsExternal + IsExternal = se.IsExternal, + IsGenerated = se.IsGenerated, + AccessLevel = se.AccessLevel }; codeStructure.Nodes.Add(element.Id, element); } diff --git a/CSharpCodeAnalyst/Persistence/Dto/SerializableCodeElement.cs b/CSharpCodeAnalyst/Persistence/Dto/SerializableCodeElement.cs index 48d81f4e..a324fa48 100644 --- a/CSharpCodeAnalyst/Persistence/Dto/SerializableCodeElement.cs +++ b/CSharpCodeAnalyst/Persistence/Dto/SerializableCodeElement.cs @@ -1,4 +1,4 @@ -using CSharpCodeAnalyst.CodeGraph.Graph; +using CSharpCodeAnalyst.CodeGraph.Graph; namespace CSharpCodeAnalyst.Persistence.Dto; @@ -10,7 +10,9 @@ public class SerializableCodeElement( CodeElementType elementType, List sourceLocations, HashSet attributes, - bool isExternal = false) + bool isExternal = false, + AccessLevel accessLevel = AccessLevel.Unknown, + bool isGenerated = false) { public string Id { get; set; } = id; public string Name { get; set; } = name; @@ -23,4 +25,16 @@ public class SerializableCodeElement( /// Whether the element belongs to a referenced assembly rather than the parsed solution. /// public bool IsExternal { get; set; } = isExternal; + + /// + /// Whether a tool wrote the element. Defaults to false, so a project file written before this + /// existed keeps loading - its elements simply carry no marking until the next parse. + /// + public bool IsGenerated { get; set; } = isGenerated; + + /// + /// How far the element can be reached from. Defaults to Unknown, so a project file written before + /// this existed keeps loading - the elements simply carry no visibility until the next parse. + /// + public AccessLevel AccessLevel { get; set; } = accessLevel; } diff --git a/CSharpCodeAnalyst/Resources/Strings.Designer.cs b/CSharpCodeAnalyst/Resources/Strings.Designer.cs index da564b4a..1fd8f493 100644 --- a/CSharpCodeAnalyst/Resources/Strings.Designer.cs +++ b/CSharpCodeAnalyst/Resources/Strings.Designer.cs @@ -1,4 +1,4 @@ -//------------------------------------------------------------------------------ +//------------------------------------------------------------------------------ // // This code was generated by a tool. // @@ -724,6 +724,15 @@ public static string DeselectAllVisible_Label { } } + /// + /// Looks up a localized string similar to Copy table as CSV. + /// + public static string DynamicGrid_CopyAsCsv { + get { + return ResourceManager.GetString("DynamicGrid_CopyAsCsv", resourceCulture); + } + } + /// /// Looks up a localized string similar to Error while loading data!. /// @@ -2783,8 +2792,9 @@ public static string SearchingCycles_Message { /// Looks up a localized string similar to Search in code element full name. /// ///Logical operations: space = AND, '|' = OR + ///Exclude a term = prefix it with '-' (e.g. "-Strings." or "-type:property") ///Search for type = type:xxx - ///Search for internal code elements = source:intern + ///Search for internal code elements = source:intern ///Search for external code elements = source:extern ///Search with resharper style = Use at least one uppercase character in a search term.. /// @@ -2976,24 +2986,6 @@ public static string Settings_IncludeExternalCode_Tooltip { } } - /// - /// Looks up a localized string similar to Include _generated code. - /// - public static string Settings_IncludeGeneratedCode { - get { - return ResourceManager.GetString("Settings_IncludeGeneratedCode", resourceCulture); - } - } - - /// - /// Looks up a localized string similar to Include source-generated code (e.g. [ObservableProperty], [RelayCommand]) when parsing the solution. Without this, calls to generated members appear as a relationship to the containing type.. - /// - public static string Settings_IncludeGeneratedCode_Tooltip { - get { - return ResourceManager.GetString("Settings_IncludeGeneratedCode_Tooltip", resourceCulture); - } - } - /// /// Looks up a localized string similar to Performance Settings. /// @@ -3328,7 +3320,7 @@ public static string TooMuchElementsTitle { /// ///Logical operations: space = AND, '|' = OR ///Search for type = type:xxx - ///Search for internal code elements = source:intern + ///Search for internal code elements = source:intern ///Search for external code elements = source:extern ///Search with resharper style = Use at least one uppercase character in a search term.. /// diff --git a/CSharpCodeAnalyst/Resources/Strings.resx b/CSharpCodeAnalyst/Resources/Strings.resx index 44af5c86..e8155ae6 100644 --- a/CSharpCodeAnalyst/Resources/Strings.resx +++ b/CSharpCodeAnalyst/Resources/Strings.resx @@ -1,4 +1,4 @@ - +