Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Value forms for name are less special, can be overridden #867

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions src/Microsoft.TemplateEngine.Core/Matching/Trie.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,20 @@ public void AddPath(byte[] path, T terminal)
next.Terminals = new List<T>();
}

next.Terminals.Add(terminal);
int sameMatcherIndex = next.Terminals.FindIndex(t => t.Start == terminal.Start && t.End == terminal.End);

if (sameMatcherIndex > -1)
{ // this matching is identical to another terminal already added to the trie. Overwrite it.
next.Terminals[sameMatcherIndex] = terminal;
}
else
{
next.Terminals.Add(terminal);
}
}

current = next.NextNodes;
}
}
}
}
}
5 changes: 1 addition & 4 deletions src/Microsoft.TemplateEngine.Core/Matching/TrieEvaluator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,7 @@ public bool TryGetNext(bool isFinal, ref int sequenceNumber, out TerminalLocatio

if (start >= _lastReturnedTerminalEndSequenceNumber)
{
// Take the terminal for the match that started first & is after the end of the previous match
// If multiple matches start at the same place, take the one that ends last.
// If multiple matches start and end at the same place, take the later one in the EncounteredTerminals list (last in wins)
if (start < minNonTerminatedPathStart && (best == null || start < minTerminalStart || start == minTerminalStart && terminal.End >= best.End))
if (start < minNonTerminatedPathStart && (best == null || start < minTerminalStart || start == minTerminalStart && terminal.End > best.End))
{
bestPath = i;
best = terminal;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
using System;
using System.Collections.Generic;
using Microsoft.TemplateEngine.Abstractions;
using Microsoft.TemplateEngine.Orchestrator.RunnableProjects.ValueForms;
using Microsoft.TemplateEngine.Utils;
using Newtonsoft.Json.Linq;
using Microsoft.TemplateEngine.Orchestrator.RunnableProjects.ValueForms;

namespace Microsoft.TemplateEngine.Orchestrator.RunnableProjects
{
Expand Down Expand Up @@ -108,10 +108,12 @@ public static ISymbolModel FromJObject(JObject jObject, IParameterSymbolLocaliza

if (!jObject.TryGetValue(nameof(symbol.Forms), StringComparison.OrdinalIgnoreCase, out JToken formsToken) || !(formsToken is JObject formsObject))
{
symbol.Forms = SymbolValueFormsModel.Default; // no value forms explicitly defined, use the default ("identity")
// no value forms explicitly defined, use the default ("identity")
symbol.Forms = SymbolValueFormsModel.Default;
}
else
{
// the config defines forms for the symbol. Use them.
symbol.Forms = SymbolValueFormsModel.FromJObject(formsObject);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -925,7 +925,12 @@ private static ISymbolModel SetupDefaultNameSymbol(string sourceName)
""description"": ""The default name symbol"",
""datatype"": ""string"",
""forms"": {
""global"": [ ""identity"", ""safe_name"", ""lower_safe_name"", ""safe_namespace"", ""lower_safe_namespace""]
""global"": [ """ + IdentityValueForm.FormName
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think interpolation would be cleaner here, the brace doubling everywhere else might make it not be though

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went back & forth on how to best format this too. Offline we decided to keep as is.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible good place for interpolation

+ @""", """ + DefaultSafeNameValueFormModel.FormName
+ @""", """ + DefaultLowerSafeNameValueFormModel.FormName
+ @""", """ + DefaultSafeNamespaceValueFormModel.FormName
+ @""", """ + DefaultLowerSafeNamespaceValueFormModel.FormName
+ @"""]
}
");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public static IValueForm GetForm(string name, JObject obj)

if (!FormLookup.TryGetValue(identifier, out IValueForm value))
{
return FormLookup["identity"].FromJObject(name, obj);
return FormLookup[IdentityValueForm.FormName].FromJObject(name, obj);
}

return value.FromJObject(name, obj);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ namespace Microsoft.TemplateEngine.Orchestrator.RunnableProjects.ValueForms
{
public class DefaultLowerSafeNameValueFormModel : DefaultSafeNameValueFormModel
{
public override string Identifier => "lower_safe_name";
public new static readonly string FormName = "lower_safe_name";

public override string Identifier => FormName;

public override string Process(IReadOnlyDictionary<string, IValueForm> forms, string value)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ namespace Microsoft.TemplateEngine.Orchestrator.RunnableProjects.ValueForms
{
public class DefaultLowerSafeNamespaceValueFormModel : DefaultSafeNamespaceValueFormModel
{
public override string Identifier => "lower_safe_namespace";
public new static readonly string FormName = "lower_safe_namespace";

public override string Identifier => FormName;

public override string Process(IReadOnlyDictionary<string, IValueForm> forms, string value)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ public DefaultSafeNameValueFormModel()
{
}

public virtual string Identifier => "safe_name";
public static readonly string FormName = "safe_name";

public virtual string Identifier => FormName;

public virtual string Name => Identifier;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ public DefaultSafeNamespaceValueFormModel()
{
}

public virtual string Identifier => "safe_namespace";
public static readonly string FormName = "safe_namespace";

public virtual string Identifier => FormName;

public string Name => Identifier;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ namespace Microsoft.TemplateEngine.Orchestrator.RunnableProjects.ValueForms
{
public class IdentityValueForm : IValueForm
{
public string Identifier => "identity";
public static readonly string FormName = "identity";

public string Identifier => FormName;

public string Name { get; }

Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
using System;
using System.Collections.Generic;
using System.Linq;
using Microsoft.TemplateEngine.Utils;
using Newtonsoft.Json.Linq;

namespace Microsoft.TemplateEngine.Orchestrator.RunnableProjects.ValueForms
{
public class SymbolValueFormsModel
{
private static readonly string IdentityValueFormName = IdentityValueForm.FormName;

public static SymbolValueFormsModel Empty { get; } = new SymbolValueFormsModel(Empty<string>.List.Value);

// by default, symbols get the "identity" value form, for a direct replacement
public static SymbolValueFormsModel Default { get; } = new SymbolValueFormsModel(new List<string>() { "identity" });
public static SymbolValueFormsModel Default { get; } = new SymbolValueFormsModel(new List<string>() { IdentityValueFormName });

public IReadOnlyList<string> GlobalForms { get; }

Expand All @@ -18,9 +22,56 @@ private SymbolValueFormsModel(IReadOnlyList<string> globalForms)
GlobalForms = globalForms;
}

public static SymbolValueFormsModel FromJObject(JObject obj)
// Sets up the value forms for a symbol, based on configuration from template.json
// There are two acceptable configuration formats for each forms specification.
//
// Note: in the examples below, "global" is used. But we'll be extending this to allow
// conditional forms, which will have other names.
// The same format will be used for other named form definitions.
//
// Simple:
// "forms": {
// "global": [ <strings representing value form names> ]
// }
//
// Detailed:
// "forms" {
// "global": {
// "forms": [ <strings representing value form names> ],
// "addIdentity": <bool default true>,
// // other future extensions, e.g. conditionals
// },
//
// If the symbol doesn't include an "identity" form and the addIdentity flag isn't false,
// an identity specification is added to the beginning of the symbol's value form list.
// If there is an identity form listed, its position remains intact irrespective of the addIdentity flag.
public static SymbolValueFormsModel FromJObject(JObject configJson)
{
IReadOnlyList<string> globalForms = obj.ArrayAsStrings("global");
JToken globalConfig = configJson.Property("global").Value;
List<string> globalForms;
bool addIdentity;
if (globalConfig.Type == JTokenType.Array)
{
// config is just an array of form names.
globalForms = globalConfig.ArrayAsStrings().ToList();
addIdentity = true; // default value
}
else if (globalConfig.Type == JTokenType.Object)
{
// config is an object.
globalForms = globalConfig.ArrayAsStrings("forms").ToList();
addIdentity = globalConfig.ToBool("addIdentity");
}
else
{
throw new Exception("Malformed global value forms.");
}

if (addIdentity && !globalForms.Contains(IdentityValueFormName, StringComparer.OrdinalIgnoreCase))
{
globalForms.Insert(0, IdentityValueFormName);
}

return new SymbolValueFormsModel(globalForms);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Generic;
using System.IO;
using Microsoft.TemplateEngine.Core.Contracts;
Expand Down Expand Up @@ -97,6 +97,29 @@ public void VerifyOperationTrieFindsTokenAtEnd()
Assert.Equal(buffer.Length, currentBufferPosition);
}

[Fact(DisplayName = nameof(VerifyLastInWinsForIdenticalMatching))]
public void VerifyLastInWinsForIdenticalMatching()
{
OperationTrie trie = OperationTrie.Create(new IOperation[]
{
new MockOperation("TestOp1", null, true, TokenConfig.LiteralToken(new byte[] { 5, 5, 5 })),
new MockOperation("TestOp2", null, true, TokenConfig.LiteralToken(new byte[] { 2, 3, 4, 5 })),
new MockOperation("TestOp3", null, true, TokenConfig.LiteralToken(new byte[] { 7, 7, 7 })),
new MockOperation("TestOp4", null, true, TokenConfig.LiteralToken(new byte[] { 9, 9, 9, 9 }),
TokenConfig.LiteralToken(new byte[] { 2, 3, 4, 5 })
),
});

byte[] buffer = { 9, 8, 9, 8, 7, 2, 3, 4, 5 };
int currentBufferPosition = 0;
IOperation match = trie.GetOperation(buffer, buffer.Length, ref currentBufferPosition, out int token);

Assert.NotNull(match);
Assert.Equal("TestOp4", match.Id);
Assert.Equal(1, token);
Assert.Equal(buffer.Length, currentBufferPosition);
}

private class OperationTrie : Trie<OperationTerminal>
{
public static OperationTrie Create(IEnumerable<IOperation> operations)
Expand Down
Loading