diff --git a/MonoDevelop.MSBuild.Editor/DisplayElementFactory.cs b/MonoDevelop.MSBuild.Editor/DisplayElementFactory.cs index 10da4b78..f1d24d79 100644 --- a/MonoDevelop.MSBuild.Editor/DisplayElementFactory.cs +++ b/MonoDevelop.MSBuild.Editor/DisplayElementFactory.cs @@ -368,7 +368,7 @@ KnownImages GetPackageImageId (FeedKind kind) case TargetInfo: return isPrivate ? KnownImages.MSBuildTargetPrivate : KnownImages.MSBuildTarget; case MetadataInfo: - return isPrivate ? KnownImages.MSBuildMetadata : KnownImages.MSBuildMetadataPrivate; + return isPrivate ? KnownImages.MSBuildMetadataPrivate : KnownImages.MSBuildMetadata; case TaskInfo: return KnownImages.MSBuildTask; case ConstantSymbol: diff --git a/MonoDevelop.MSBuild.Tests/MSBuildIdentifierTests.cs b/MonoDevelop.MSBuild.Tests/MSBuildIdentifierTests.cs new file mode 100644 index 00000000..d098cc9e --- /dev/null +++ b/MonoDevelop.MSBuild.Tests/MSBuildIdentifierTests.cs @@ -0,0 +1,98 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using MonoDevelop.MSBuild.Language; +using MonoDevelop.MSBuild.Language.Syntax; +using MonoDevelop.MSBuild.Language.Typesystem; + +using NUnit.Framework; + +namespace MonoDevelop.MSBuild.Tests; + +[TestFixture] +class MSBuildIdentifierTests +{ + [TestCase ("_FooBar", true)] + [TestCase ("FooBar", true)] + [TestCase ("fooBar", true)] + [TestCase ("foo", true)] + [TestCase ("Foo", true)] + [TestCase ("", false)] + [TestCase ("-FooBar", false)] + [TestCase (" FooBar", false)] + [TestCase ("FooBar ", false)] + [TestCase ("Foo Bar", false)] + [TestCase ("1Foo", false)] + [TestCase ("\"Foo", false)] + public void TestValidIdentifiers (string identifier, bool isValid) + { + bool actual = MSBuildIdentifier.IsValid (identifier); + Assert.AreEqual (isValid, actual); + } + + [TestCase ("IsFoo", "Is")] + [TestCase ("_IsFoo", "Is")] + [TestCase ("_HasFoo", "Has")] + [TestCase ("Foo", null)] + [TestCase ("_Foo", null)] + [TestCase ("__Foo", null)] + [TestCase (" Foo", null)] + public void TestGetPrefix (string identifier, string prefix) + { + bool success = MSBuildIdentifier.TryGetPrefix (identifier, out var actual); + Assert.AreEqual (prefix is not null, success); + Assert.AreEqual (prefix, actual); + } + + [TestCase ("FooPath", "Path")] + [TestCase ("_FooEnabled", "Enabled")] + [TestCase ("_PineappleOn", "On")] + [TestCase ("Foo", null)] + [TestCase ("Foo_", null)] + [TestCase ("_Cat", null)] + [TestCase ("__Mouse_", null)] + [TestCase ("Walrus ", null)] + public void TestGetSuffix (string identifier, string suffix) + { + bool success = MSBuildIdentifier.TryGetSuffix (identifier, out var actual); + Assert.AreEqual (suffix is not null, success); + Assert.AreEqual (suffix, actual); + } + + [TestCase ("Enable", MSBuildValueKind.Unknown)] + [TestCase ("EnableFoo", MSBuildValueKind.Bool)] + [TestCase ("_EnableFoo", MSBuildValueKind.Bool)] + [TestCase ("DisableFoo", MSBuildValueKind.Bool)] + [TestCase ("RequireFoo", MSBuildValueKind.Bool)] + [TestCase ("UseFoo", MSBuildValueKind.Bool)] + [TestCase ("AllowFoo", MSBuildValueKind.Bool)] + [TestCase ("IsFoo", MSBuildValueKind.Bool)] + [TestCase ("HasFoo", MSBuildValueKind.Bool)] + [TestCase ("FooEnabled", MSBuildValueKind.Bool)] + [TestCase ("FooDisabled", MSBuildValueKind.Bool)] + [TestCase ("FooRequired", MSBuildValueKind.Bool)] + [TestCase ("RequiredFoo", MSBuildValueKind.Unknown)] + [TestCase ("FooDependsOn", MSBuildValueKind.TargetName | MSBuildValueKind.ListSemicolon)] + [TestCase ("FooPath", MSBuildValueKind.FileOrFolder)] + [TestCase ("FooPaths", MSBuildValueKind.FileOrFolder | MSBuildValueKind.ListSemicolon)] + [TestCase ("FooDirectory", MSBuildValueKind.Folder)] + [TestCase ("FooDir", MSBuildValueKind.Folder)] + [TestCase ("FooFile", MSBuildValueKind.File)] + [TestCase ("FooFileName", MSBuildValueKind.Filename)] + [TestCase ("FooFilename", MSBuildValueKind.Filename)] + [TestCase ("FooUrl", MSBuildValueKind.Url)] + [TestCase ("FooUri", MSBuildValueKind.Url)] + [TestCase ("FooExt", MSBuildValueKind.Extension)] + [TestCase ("FooGuid", MSBuildValueKind.Guid)] + [TestCase ("FooDirectories", MSBuildValueKind.Folder | MSBuildValueKind.ListSemicolon)] + [TestCase ("FooDirs", MSBuildValueKind.Folder | MSBuildValueKind.ListSemicolon)] + [TestCase ("FooFiles", MSBuildValueKind.File | MSBuildValueKind.ListSemicolon)] + // note: these two are only valid for Property while the rest are valid for Property and Metadata + [TestCase ("Configuration", MSBuildValueKind.Configuration)] + [TestCase ("Platform", MSBuildValueKind.Platform)] + public void TestInferKind (string identifier, MSBuildValueKind kind) + { + var actual = MSBuildIdentifier.InferValueKind (identifier, MSBuildSyntaxKind.Property); + Assert.AreEqual (kind, actual); + } +} diff --git a/MonoDevelop.MSBuild/Language/Expressions/ExpressionParser.cs b/MonoDevelop.MSBuild/Language/Expressions/ExpressionParser.cs index de20ef04..22c4d828 100644 --- a/MonoDevelop.MSBuild/Language/Expressions/ExpressionParser.cs +++ b/MonoDevelop.MSBuild/Language/Expressions/ExpressionParser.cs @@ -176,7 +176,7 @@ static ExpressionNode ParseItem (string buffer, ref int offset, int endOffset, i ConsumeWhitespace (ref offset); - string name = ReadMSBuildIdentifier (buffer, ref offset, endOffset); + string name = MSBuildIdentifier.TryRead (buffer, ref offset, endOffset); if (name == null) { return new ExpressionError (baseOffset + offset, ExpressionErrorKind.ExpectingItemName, out hasError); } @@ -349,39 +349,6 @@ static string TryReadNameAsciiLettersOnly (string buffer, ref int offset, int en static bool IsAsciiLetter (char ch) => (ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z'); } - static string ReadMSBuildIdentifier (string buffer, ref int offset, int endOffset) - { - if (offset > endOffset) { - return null; - } - - int start = offset; - char ch = buffer [offset]; - if (!char.IsLetter (ch) && ch != '_') { - return null; - } - offset++; - - char lastChar = ch; - while (offset <= endOffset) { - ch = buffer [offset]; - if (!char.IsLetterOrDigit (ch) && ch != '_' && ch != '-') { - break; - } - offset++; - lastChar = ch; - } - - // Although a dash is a valid char for MSBuild identifiers, we will disallow the last char being a dash - // as this will conflict with item transform handling. - // We could probably allow this if we special cased it but I really can't see a good reason to do so. - if (lastChar == '-') { - offset--; - } - - return buffer.Substring (start, offset - start); - } - // TODO: improve and unify with the CLR identifier validation in the validator static string ReadClrIdentifier (string buffer, ref int offset, int endOffset) { @@ -720,7 +687,7 @@ static ExpressionNode ParseProperty (string buffer, ref int offset, int endOffse return new ExpressionProperty (baseOffset + start, offset - start, propRef); } } else { - string name = ReadMSBuildIdentifier (buffer, ref offset, endOffset); + string name = MSBuildIdentifier.TryRead (buffer, ref offset, endOffset); if (name == null) { return new ExpressionError (baseOffset + offset, ExpressionErrorKind.ExpectingPropertyName, out hasError); } @@ -1037,7 +1004,7 @@ static ExpressionNode ParseMetadata (string buffer, ref int offset, int endOffse ConsumeSpace (buffer, ref offset, endOffset); - string name = ReadMSBuildIdentifier (buffer, ref offset, endOffset); + string name = MSBuildIdentifier.TryRead (buffer, ref offset, endOffset); if (name == null) { return new ExpressionError (baseOffset + offset, ExpressionErrorKind.ExpectingMetadataOrItemName, out hasError); } @@ -1076,7 +1043,7 @@ out hasError ConsumeSpace (buffer, ref offset, endOffset); - string metadataName = ReadMSBuildIdentifier (buffer, ref offset, endOffset); + string metadataName = MSBuildIdentifier.TryRead (buffer, ref offset, endOffset); if (metadataName == null) { return new IncompleteExpressionError ( baseOffset + offset, offset > endOffset, ExpressionErrorKind.ExpectingMetadataName, diff --git a/MonoDevelop.MSBuild/Language/MSBuildIdentifier.cs b/MonoDevelop.MSBuild/Language/MSBuildIdentifier.cs new file mode 100644 index 00000000..09de5b28 --- /dev/null +++ b/MonoDevelop.MSBuild/Language/MSBuildIdentifier.cs @@ -0,0 +1,244 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#nullable enable + +using System; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; + +using MonoDevelop.MSBuild.Language.Syntax; +using MonoDevelop.MSBuild.Language.Typesystem; + +namespace MonoDevelop.MSBuild.Language; + +static class MSBuildIdentifier +{ + /// + /// Tries to read an MSBuild identifier from the between the and , + /// and advance the to the end of the identifier. + /// + public static string? TryRead (string buffer, ref int offset, int endOffset) + { + if (TryGetLength (buffer, offset, endOffset, out int length)) { + string val = buffer.Substring (offset, length); + offset += length; + return val; + } + return null; + } + + /// + /// Tries to read an MSBuild identifier from the between the and . + /// The will be advanced to the end of the identifier. + /// + /// + /// + /// + /// + public static bool TryGetLength (string buffer, int offset, int endOffset, out int length) + { + length = 0; + if (offset > endOffset) { + return false; + } + + int start = offset; + char ch = buffer[offset]; + if (!IsIdentifierFirstChar (ch)) { + return false; + } + offset++; + + char lastChar = ch; + while (offset <= endOffset) { + ch = buffer[offset]; + if (!IsIdentifierChar (ch)) { + break; + } + offset++; + lastChar = ch; + } + + // Although a dash is a valid char for MSBuild identifiers, we will disallow the last char being a dash + // as this will conflict with item transform handling. + // We could probably allow this if we special cased it but I really can't see a good reason to do so. + if (lastChar == '-') { + offset--; + } + + length = offset - start; + return true; + } + + static bool IsIdentifierChar (char ch) => char.IsLetterOrDigit (ch) || ch == '_' || ch == '-'; + static bool IsIdentifierFirstChar (char ch) => char.IsLetter (ch) || ch == '_'; + static bool IsPascalComponentStart (char ch) => char.IsUpper (ch); + static bool IsPascalComponentChar (char ch) => char.IsLetterOrDigit (ch); + + public static bool IsValid (string identifier) => TryGetLength (identifier, 0, identifier.Length - 1, out int length) && length == identifier.Length; + + /// + /// Gets a PascalCase prefix from the identifier + /// + public static bool TryGetPrefix (string identifier, [NotNullWhen(true)] out string? prefix) + { + prefix = null; + + int idx = 0; + + if (identifier.Length < 4) { + // a prefix requires two components so at minimum "AaBa" + return false; + } + + // ignore the _ visibility convention prefix + if (identifier[idx] == '_') { + idx++; + } + + // prefix must start with a PascalCase component start char + if (!IsPascalComponentStart (identifier[idx])) { + return false; + } + int prefixStart = idx; + + idx++; + + while (idx < identifier.Length) { + char ch = identifier [idx]; + if (!IsPascalComponentChar (ch)) { + return false; + } + // only return the prefix when we found the start of a + // preceding PascalCase component, else it's not prefixing anything + if (IsPascalComponentStart (ch)) { + int prefixLength = idx - prefixStart; + if (prefixLength == 0) { + return false; + } + prefix = identifier.Substring (prefixStart, prefixLength); + return true; + } + idx++; + } + + return false; + } + + /// + /// Gets a PascalCase suffix from the identifier + /// + public static bool TryGetSuffix (string identifier, [NotNullWhen (true)] out string? suffix) + { + suffix = null; + + int endIdx = identifier.Length - 1; + int idx = endIdx; + if (idx < 0) { + return false; + } + + string? possibleSuffix = null; + + while (idx >= 0) { + char ch = identifier[idx]; + if (!IsPascalComponentChar (ch)) { + return false; + } + if (IsPascalComponentStart (ch)) { + if (idx == endIdx) { + // a suffix is more than just one uppercase char + suffix = null; + return false; + } + if (possibleSuffix == null) { + possibleSuffix = identifier.Substring (idx, endIdx - idx + 1); + } else { + // only return the suffix when we found the start of a + // preceding PascalCase component, else it's not suffixing anything + suffix = possibleSuffix; + return true; + } + } + idx--; + } + + suffix = null; + return false; + } + + /// + /// Try to infer the value kind from an identifier. + /// + public static MSBuildValueKind InferValueKind (string name, MSBuildSyntaxKind syntaxKind) + { + if (syntaxKind == MSBuildSyntaxKind.Property || syntaxKind == MSBuildSyntaxKind.Metadata) { + if (TryGetPrefix (name, out var prefix)) { + if (knownPrefixes.TryGetValue (prefix, out var valuekind)) { + return valuekind; + } + } + if (TryGetSuffix (name, out var suffix)) { + if (knownSuffixes.TryGetValue (suffix, out var valueKind)) { + return valueKind; + } + // these suffixes have multiple PascalCase components so must be handled differently + if (EndsWith ("DependsOn")) { + return MSBuildValueKind.TargetName.AsList (); + } + if (EndsWith ("FileName")) { + return MSBuildValueKind.Filename; + } + } + } + + //make sure these work even if the common targets schema isn't loaded + if (syntaxKind == MSBuildSyntaxKind.Property) { + if (Equals ("Configuration")) { + return MSBuildValueKind.Configuration; + } + if (Equals ("Platform")) { + return MSBuildValueKind.Platform; + } + } + + if (syntaxKind == MSBuildSyntaxKind.Item) { + return MSBuildValueKind.Unknown.AsList (); + } + + return MSBuildValueKind.Unknown; + + bool EndsWith (string suffix) => name.EndsWith (suffix, StringComparison.OrdinalIgnoreCase); + bool Equals (string value) => name.Equals (value, StringComparison.OrdinalIgnoreCase); + } + + static readonly Dictionary knownPrefixes = new () { + { "Enable", MSBuildValueKind.Bool }, + { "Disable", MSBuildValueKind.Bool }, + { "Require", MSBuildValueKind.Bool }, + { "Use", MSBuildValueKind.Bool }, + { "Allow", MSBuildValueKind.Bool }, + { "Is", MSBuildValueKind.Bool }, + { "Has", MSBuildValueKind.Bool } + }; + + static readonly Dictionary knownSuffixes = new () { + { "Enabled", MSBuildValueKind.Bool }, + { "Disabled", MSBuildValueKind.Bool }, + { "Required", MSBuildValueKind.Bool }, + { "Path", MSBuildValueKind.FileOrFolder }, + { "Paths", MSBuildValueKind.FileOrFolder.AsList () }, + { "Directory", MSBuildValueKind.Folder }, + { "Dir", MSBuildValueKind.Folder }, + { "File", MSBuildValueKind.File }, + { "Filename", MSBuildValueKind.Filename }, + { "Uri", MSBuildValueKind.Url }, + { "Url", MSBuildValueKind.Url }, + { "Ext", MSBuildValueKind.Extension }, + { "Guid", MSBuildValueKind.Guid }, + { "Directories", MSBuildValueKind.Folder.AsList () }, + { "Dirs", MSBuildValueKind.Folder.AsList () }, + { "Files", MSBuildValueKind.File.AsList () } + }; +} diff --git a/MonoDevelop.MSBuild/Language/MSBuildInferredSchema.cs b/MonoDevelop.MSBuild/Language/MSBuildInferredSchema.cs index 249d5d07..cab79fa4 100644 --- a/MonoDevelop.MSBuild/Language/MSBuildInferredSchema.cs +++ b/MonoDevelop.MSBuild/Language/MSBuildInferredSchema.cs @@ -53,6 +53,7 @@ public bool IsPrivate (string name) ItemInfo _ => Items.ContainsKey (info.Name), TaskInfo _ => Tasks.ContainsKey (info.Name), TargetInfo _ => Targets.ContainsKey (info.Name), + MetadataInfo m => MetadataUsage.ContainsKey((m.Item?.Name, m.Name)), _ => false }; @@ -212,7 +213,7 @@ void CollectItem (string itemName, ReferenceUsage usage) } usage |= existingUsage; } else { - var kind = InferValueKindFromName (itemName, MSBuildSyntaxKind.Item); + var kind = MSBuildIdentifier.InferValueKind (itemName, MSBuildSyntaxKind.Item); Items.Add (itemName, new InferredItemInfo (itemName, kind)); } ItemUsage[itemName] = usage; @@ -230,7 +231,7 @@ void CollectProperty (string propertyName, ReferenceUsage usage) } usage |= existingUsage; } else if (!MSBuildIntrinsics.Properties.ContainsKey (propertyName)) { - var kind = InferValueKindFromName (propertyName, MSBuildSyntaxKind.Property); + var kind = MSBuildIdentifier.InferValueKind (propertyName, MSBuildSyntaxKind.Property); Properties.Add (propertyName, new InferredPropertyInfo (propertyName, kind)); } PropertyUsage[propertyName] = usage; @@ -267,13 +268,13 @@ void CollectMetadata (string itemName, string metadataName, ReferenceUsage usage usage |= existingUsage; } else if (!MSBuildIntrinsics.Metadata.ContainsKey (metadataName)) { var item = Items[itemName]; - var kind = InferValueKindFromName (metadataName, MSBuildSyntaxKind.Metadata); + var kind = MSBuildIdentifier.InferValueKind (metadataName, MSBuildSyntaxKind.Metadata); item.Metadata.Add (metadataName, new InferredMetadataInfo (metadataName, kind, item: item)); } MetadataUsage[(itemName, metadataName)] = usage; } - class InferredMetadataInfo (string name, MSBuildValueKind inferredKind, ItemInfo item) : MetadataInfo (name, null, valueKind: inferredKind, item: item) + class InferredMetadataInfo (string name, MSBuildValueKind inferredKind, ItemInfo item) : MetadataInfo (name, null, valueKind: inferredKind, item: item), IInferredSymbol { } @@ -530,79 +531,11 @@ void CollectComparisonProperty (ExpressionProperty prop, string value) public static MSBuildValueKind InferValueKindFromName (ISymbol symbol) => symbol switch { ItemInfo item => MSBuildValueKind.FileOrFolder.AsList (), - MetadataInfo metadata => InferValueKindFromName (metadata.Name, MSBuildSyntaxKind.Metadata), - PropertyInfo property => InferValueKindFromName (property.Name, MSBuildSyntaxKind.Property), + MetadataInfo metadata => MSBuildIdentifier.InferValueKind (metadata.Name, MSBuildSyntaxKind.Metadata), + PropertyInfo property => MSBuildIdentifier.InferValueKind (property.Name, MSBuildSyntaxKind.Property), _ => MSBuildValueKind.Unknown }; - public static MSBuildValueKind InferValueKindFromName (string name, MSBuildSyntaxKind kind) - { - if (kind == MSBuildSyntaxKind.Property || kind == MSBuildSyntaxKind.Metadata) { - if (StartsWith ("Enable") - || StartsWith ("Disable") - || StartsWith ("Require") - || StartsWith ("Use") - || StartsWith ("Allow") - || EndsWith ("Enabled") - || EndsWith ("Disabled") - || EndsWith ("Required")) { - return MSBuildValueKind.Bool; - } - if (EndsWith ("DependsOn")) { - return MSBuildValueKind.TargetName.AsList (); - } - if (EndsWith ("Path")) { - return MSBuildValueKind.FileOrFolder; - } - if (EndsWith ("Paths")) { - return MSBuildValueKind.FileOrFolder.AsList (); - } - if (EndsWith ("Directory") - || EndsWith ("Dir")) { - return MSBuildValueKind.Folder; - } - if (EndsWith ("File")) { - return MSBuildValueKind.File; - } - if (EndsWith ("FileName")) { - return MSBuildValueKind.Filename; - } - if (EndsWith ("Url")) { - return MSBuildValueKind.Url; - } - if (EndsWith ("Ext")) { - return MSBuildValueKind.Extension; - } - if (EndsWith ("Guid")) { - return MSBuildValueKind.Guid; - } - if (EndsWith ("Directories") || EndsWith ("Dirs")) { - return MSBuildValueKind.Folder.AsList (); - } - if (EndsWith ("Files")) { - return MSBuildValueKind.File.AsList (); - } - } - - //make sure these work even if the common targets schema isn't loaded - if (kind == MSBuildSyntaxKind.Property) { - if (Equals ("Configuration")) { - return MSBuildValueKind.Configuration; - } - if (Equals ("Platform")) { - return MSBuildValueKind.Platform; - } - } - - return MSBuildValueKind.Unknown; - - bool StartsWith (string prefix) => name.StartsWith (prefix, StringComparison.OrdinalIgnoreCase) - && name.Length > prefix.Length - && char.IsUpper (name[prefix.Length]); - bool EndsWith (string suffix) => name.EndsWith (suffix, StringComparison.OrdinalIgnoreCase); - bool Equals (string value) => name.Equals (value, StringComparison.OrdinalIgnoreCase); - } - [LoggerMessage (EventId = 0, Level = LogLevel.Error, Message = "Internal error in schema inference for {filename}")] static partial void LogInternalError (ILogger logger, UserIdentifiableFileName filename, Exception ex);