Code Quality: Generative resource management and refactoring (#15662)

Author: XTorLukas

Files.sln

@@ -626,4 +626,4 @@ Global
 	GlobalSection(ExtensibilityGlobals) = postSolution
 		SolutionGuid = {0E62043C-A7A1-4982-9EC9-4CDB2939B776}
 	EndGlobalSection
-EndGlobal
+EndGlobal

src/Files.App/Files.App.csproj

@@ -133,5 +133,9 @@
         <TrimmerRootAssembly Include="CommunityToolkit.WinUI.UI.Controls.Media" />
         <TrimmerRootAssembly Include="CommunityToolkit.WinUI.UI.Controls.Primitives" />
     </ItemGroup>
+
+	<ItemGroup>
+		<AdditionalFiles Include="Strings\en-US\Resources.resw" />
+	</ItemGroup>
 
 </Project>

src/Files.App/Helpers/StringsHelper.cs

@@ -0,0 +1,12 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace Files.App.Helpers
+{
+	public sealed partial class Strings
+	{
+	}
+}

src/Files.App/ViewModels/Properties/Items/FileProperty.cs

@@ -231,7 +231,7 @@ private IValueConverter GetConverter()
 			{
 				try
 				{
-					return EnumeratedList.TryGetValue(Convert.ToInt32(Value), out var value) ? 
+					return EnumeratedList.TryGetValue(Convert.ToInt32(Value), out var value) ?
 						value.GetLocalizedResource() : null;
 				}
 				catch

src/Files.Core.SourceGenerator/AnalyzerReleases.Unshipped.md

@@ -7,3 +7,5 @@
 Rule ID | Category | Severity | Notes
 --------|----------|----------|--------------------
 FSG1001 |  Design  |   Error  | FSG1001_Files.Core.SourceGenerator
+FSG1002 | Refactoring | Warning | FSG1002_Files.Core.SourceGenerator
+FSG1003 | FileGeneration | Error | FSG1003_Files.Core.SourceGenerator

src/Files.Core.SourceGenerator/Analyzers/StringsPropertyAnalyzer.cs

@@ -0,0 +1,98 @@
+// Copyright (c) 2024 Files Community
+// Licensed under the MIT License. See the LICENSE.
+
+using static Files.Core.SourceGenerator.Constants.DiagnosticDescriptors;
+using static Files.Core.SourceGenerator.Constants.StringsPropertyGenerator;
+
+namespace Files.Core.SourceGenerator.Analyzers
+{
+	/// <summary>
+	/// Analyzer that detects if string literals can be replaced with constants from the <c>Strings</c> class.
+	/// </summary>
+	[DiagnosticAnalyzer(LanguageNames.CSharp)]
+	internal sealed class StringsPropertyAnalyzer : DiagnosticAnalyzer
+	{
+		/// <summary>
+		/// Represents a collection of constant string names and their corresponding values.
+		/// </summary>
+		internal static FrozenDictionary<string, string?>? StringsConstants { get; private set; } = null;
+
+		/// <summary>
+		/// Gets the supported diagnostics for this analyzer.
+		/// </summary>
+		public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [FSG1002];
+
+		/// <summary>
+		/// Initializes the analyzer and registers the syntax node action.
+		/// </summary>
+		/// <param name="context">The analysis context.</param>
+		public override void Initialize(AnalysisContext context)
+		{
+			context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+			context.EnableConcurrentExecution();
+			context.RegisterSyntaxNodeAction(AnalyzeNode, SyntaxKind.StringLiteralExpression, SyntaxKind.InterpolatedStringText);
+		}
+
+		/// <summary>
+		/// Analyzes the syntax node to detect if a string literal can be replaced with a constant from the Strings class.
+		/// </summary>
+		/// <param name="context">The syntax node analysis context.</param>
+		private void AnalyzeNode(SyntaxNodeAnalysisContext context)
+		{
+			var literalValue = string.Empty;
+			Location locationValue = default!;
+			SyntaxNode? parent = null;
+
+			if (context.Node is InterpolatedStringTextSyntax interpolatedStringText)
+			{
+				literalValue = interpolatedStringText.TextToken.ValueText;
+				locationValue = interpolatedStringText.GetLocation();
+				parent = interpolatedStringText.Parent?.Parent;
+			}
+
+			if (context.Node is LiteralExpressionSyntax literalExpression)
+			{
+				literalValue = literalExpression.Token.ValueText;
+				locationValue = literalExpression.GetLocation();
+				parent = literalExpression.Parent;
+			}
+
+			if (string.IsNullOrEmpty(literalValue) ||
+				parent is not MemberAccessExpressionSyntax memberAccessExpression ||
+				!LocalizedMethodNames.Contains(memberAccessExpression.Name.Identifier.Text))
+				return;
+
+			if (StringsConstants is null)
+			{
+				var semanticModel = context.SemanticModel;
+				var compilation = semanticModel.Compilation;
+
+				// Get the type symbol for the Strings class
+				var stringsTypeSymbol = compilation.GetTypeByMetadataName(StringsMetadataName);
+
+				if (stringsTypeSymbol == null)
+					return;
+
+				// Extract constants from the Strings class
+				StringsConstants = stringsTypeSymbol.GetMembers()
+					.OfType<IFieldSymbol>()
+					.Where(f => f.IsConst)
+					.ToDictionary(f => f.Name, f => f.ConstantValue?.ToString())
+					.ToFrozenDictionary();
+			}
+
+			// Check if the literal value matches any of the constants
+			var match = StringsConstants.FirstOrDefault(pair => pair.Value == literalValue);
+
+			if (!match.Equals(default(KeyValuePair<string, string>)))
+			{
+				var properties = ImmutableDictionary<string, string>
+					.Empty
+					.Add(ConstantNameProperty, match.Key);
+
+				var diagnostic = Diagnostic.Create(FSG1002, locationValue, properties!, literalValue, match.Key);
+				context.ReportDiagnostic(diagnostic);
+			}
+		}
+	}
+}

src/Files.Core.SourceGenerator/CodeFixProviders/StringsPropertyCodeFixProvider.cs

@@ -0,0 +1,64 @@
+// Copyright (c) 2024 Files Community
+// Licensed under the MIT License. See the LICENSE.
+
+using static Files.Core.SourceGenerator.Constants.DiagnosticDescriptors;
+using static Files.Core.SourceGenerator.Constants.StringsPropertyGenerator;
+
+namespace Files.Core.SourceGenerator.CodeFixProviders
+{
+	/// <summary>
+	/// Code fix provider that replaces string literals with constants from the <c>Strings</c> class.
+	/// </summary>
+	[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(StringsPropertyCodeFixProvider)), Shared]
+	internal sealed class StringsPropertyCodeFixProvider : CodeFixProvider
+	{
+		/// <summary>
+		/// Gets a list of diagnostic IDs that this provider can fix.
+		/// </summary>
+		public sealed override ImmutableArray<string> FixableDiagnosticIds => [FSG1002.Id];
+
+		/// <summary>
+		/// Gets the fix all provider for this code fix provider.
+		/// </summary>
+		/// <returns>A <see cref="FixAllProvider"/>.</returns>
+		public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;
+
+		/// <summary>
+		/// Registers code fixes for the specified diagnostic.
+		/// </summary>
+		/// <param name="context">The context for the code fix.</param>
+		public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
+		{
+			var diagnostic = context.Diagnostics.First();
+
+			var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
+			if (root == null)
+				return;
+
+			var diagnosticSpan = diagnostic.Location.SourceSpan;
+
+			SyntaxNode? node = null;
+
+			if (root.FindNode(diagnosticSpan) is LiteralExpressionSyntax literalExpression)
+				node = literalExpression;
+
+			if (root.FindNode(diagnosticSpan) is InterpolatedStringTextSyntax interpolatedStringText)
+				node = interpolatedStringText.Parent;
+
+			if (node is null)
+				return;
+
+			var constantName = diagnostic.Properties[ConstantNameProperty];
+			var newExpression = SyntaxFactory.ParseExpression($"{StringsClassName}.{constantName}").WithTriviaFrom(node);
+			var newRoot = root.ReplaceNode(node, newExpression);
+			var newDocument = context.Document.WithSyntaxRoot(newRoot);
+
+			context.RegisterCodeFix(
+				CodeAction.Create(
+					CodeFixProviderTitle,
+					c => Task.FromResult(newDocument),
+					null),
+				diagnostic);
+		}
+	}
+}

src/Files.Core.SourceGenerator/Constants.cs

@@ -0,0 +1,102 @@
+// Copyright (c) 2024 Files Community
+// Licensed under the MIT License. See the LICENSE.
+
+namespace Files.Core.SourceGenerator
+{
+	/// <summary>
+	/// Contains various constants used within the source generator.
+	/// </summary>
+	internal class Constants
+	{
+		/// <summary>
+		/// Contains diagnostic descriptors used for error reporting.
+		/// </summary>
+		internal class DiagnosticDescriptors
+		{
+			/// <summary>
+			/// Diagnostic descriptor for unsupported types in Windows Registry.
+			/// </summary>
+			internal static readonly DiagnosticDescriptor FSG1001 = new(
+				id: nameof(FSG1001),
+				title: "Types that are not supported by Windows Registry",
+				messageFormat: "Type '{0}' is not supported by Windows Registry",
+				category: "Design",
+				defaultSeverity: DiagnosticSeverity.Error,
+				isEnabledByDefault: true);
+
+			/// <summary>
+			/// Diagnostic descriptor for a refactoring suggestion to replace string literals with constants from the Strings class.
+			/// </summary>
+			internal static readonly DiagnosticDescriptor FSG1002 = new(
+				id: nameof(FSG1002),
+				title: "String literal can be replaced with constant",
+				messageFormat: $"Replace '{{0}}' with '{StringsPropertyGenerator.StringsClassName}.{{1}}'",
+				category: "Refactoring",
+				defaultSeverity: DiagnosticSeverity.Warning,
+				isEnabledByDefault: true,
+				description: $"Detects string literals that can be replaced with constants from the {StringsPropertyGenerator.StringsClassName} class.");
+
+			/// <summary>
+			/// Diagnostic descriptor for a scenario where multiple files with the same name are detected.
+			/// </summary>
+			internal static readonly DiagnosticDescriptor FSG1003 = new(
+				id: nameof(FSG1003),
+				title: "Multiple files with the same name detected",
+				messageFormat: "Multiple files named '{0}' were detected. Ensure all generated localization string files have unique names.",
+				category: "FileGeneration",
+				defaultSeverity: DiagnosticSeverity.Error,
+				isEnabledByDefault: true,
+				description: "This diagnostic detects cases where multiple localization string files are being generated with the same name," +
+				"which can cause conflicts and overwrite issues.");
+
+		}
+
+		/// <summary>
+		/// Contains constants related to DependencyProperty generation.
+		/// </summary>
+		internal class DependencyPropertyGenerator
+		{
+			/// <summary>
+			/// The name of the attribute used for DependencyProperty.
+			/// </summary>
+			internal static readonly string AttributeName = "DependencyPropertyAttribute";
+		}
+
+		internal class StringsPropertyGenerator
+		{
+			/// <summary>
+			/// The name of the generated class that contains string constants.
+			/// </summary>
+			internal const string StringsClassName = "Strings";
+
+			/// <summary>
+			/// The fully qualified name of the generated metadata class that contains string constants.
+			/// </summary>
+			internal const string StringsMetadataName = $"{SourceGeneratorHelper.HelperNamespace}{StringsClassName}";
+
+			/// <summary>
+			/// The name of the property that represents the name of the constant.
+			/// </summary>
+			internal const string ConstantNameProperty = nameof(ConstantNameProperty);
+
+			/// <summary>
+			/// A collection of method names that are considered localized methods.
+			/// These methods are used to identify string literals that can be replaced with constants from the Strings class.
+			/// </summary>
+			internal static HashSet<string> LocalizedMethodNames = [
+				/* TODO: Future use only this */ "ToLocalized",
+				/* TODO: Rewrite with ToLocalized */ "GetLocalizedResource",
+				/* TODO: Rewrite with ToLocalized */ "GetLocalizedFormatResource"];
+
+			/// <summary>
+			/// The title of the code fix provider that suggests replacing string literals with constants from the Strings class.
+			/// </summary>
+			internal const string CodeFixProviderTitle = $"Replace with constant from {StringsClassName}";
+
+			/// <summary>
+			/// Represents a character used as a separator in constant names.
+			/// </summary>
+			internal const char ConstantSeparator = '/';
+		}
+	}
+}

src/Files.Core.SourceGenerator/Data/ParserItem.cs

@@ -0,0 +1,35 @@
+// Copyright (c) 2024 Files Community
+// Licensed under the MIT License. See the LICENSE.
+
+namespace Files.Core.SourceGenerator.Data
+{
+	/// <summary>
+	/// Represents an item used for parsing data.
+	/// </summary>
+	internal class ParserItem
+	{
+		/// <summary>
+		/// Gets or sets the key of the item.
+		/// </summary>
+		/// <remarks>
+		/// This property is required and cannot be null or empty.
+		/// </remarks>
+		internal required string Key { get; set; } = default!;
+
+		/// <summary>
+		/// Gets or sets the value of the item.
+		/// </summary>
+		/// <remarks>
+		/// The default value is an empty string.
+		/// </remarks>
+		internal string Value { get; set; } = string.Empty;
+
+		/// <summary>
+		/// Gets or sets the comment associated with the item.
+		/// </summary>
+		/// <remarks>
+		/// The default value is null, indicating no comment.
+		/// </remarks>
+		internal string? Comment { get; set; } = null;
+	}
+}