Skip to content

[generator] Fix invalid parsing of complex generic types. #729

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

Merged
merged 1 commit into from
Sep 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
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
55 changes: 55 additions & 0 deletions tests/generator-Tests/Unit-Tests/CodeGenerationOptionsTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
using System;
using MonoDroid.Generation;
using NUnit.Framework;

namespace generatortests
{
[TestFixture]
public class CodeGenerationOptionsTests
{
[Test]
public void GetOutputNameUseGlobal ()
{
var opt = new CodeGenerationOptions { UseGlobal = true };

Assert.AreEqual (string.Empty, opt.GetOutputName (string.Empty));
Assert.AreEqual ("int", opt.GetOutputName ("int"));
Assert.AreEqual ("void", opt.GetOutputName ("void"));
Assert.AreEqual ("void", opt.GetOutputName ("System.Void"));
Assert.AreEqual ("params int[]", opt.GetOutputName ("params int[]"));
Assert.AreEqual ("params global::System.Object[]", opt.GetOutputName ("params System.Object[]"));
Copy link
Member

Choose a reason for hiding this comment

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

Should also check arrays-of-arrays, e.g. int[][][].

Assert.AreEqual ("int[][][]", opt.GetOutputName ("int[][][]"));
Assert.AreEqual ("global::System.Object[][][]", opt.GetOutputName ("System.Object[][][]"));

Assert.AreEqual ("global::System.Collections.Generic.List<string[]>",
opt.GetOutputName ("System.Collections.Generic.List<string[]>"));

Assert.AreEqual ("global::System.Collections.Generic.Dictionary<string, string>",
opt.GetOutputName ("System.Collections.Generic.Dictionary<string, string>"));

Assert.AreEqual ("global::System.Collections.Generic.List<global::System.Collections.Generic.List<string>>",
opt.GetOutputName ("System.Collections.Generic.List<System.Collections.Generic.List<string>>"));
Copy link
Member

@jonpryor jonpryor Sep 24, 2020

Choose a reason for hiding this comment

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

Also try intermixing arrays & generics, e.g. System.Collections.Generic.List<System.Collections.Generic.List<string>[]>[].


Assert.AreEqual ("global::System.Collections.Generic.List<global::System.Collections.Generic.Dictionary<string, global::System.Collections.Generic.Dictionary<global::System.Object, global::System.Object>>>",
opt.GetOutputName ("System.Collections.Generic.List<System.Collections.Generic.Dictionary<string, System.Collections.Generic.Dictionary<System.Object, System.Object>>>"));

Assert.AreEqual ("global::System.Collections.Generic.IList<global::Kotlin.Pair>",
opt.GetOutputName ("System.Collections.Generic.IList<Kotlin.Pair>"));

Assert.AreEqual ("global::System.Collections.Generic.IDictionary<string, global::System.Collections.Generic.IList<global::Kotlin.Pair>>",
opt.GetOutputName ("System.Collections.Generic.IDictionary<string, System.Collections.Generic.IList<Kotlin.Pair>>"));

Assert.AreEqual ("global::System.Collections.Generic.IDictionary<global::System.Collections.Generic.IList<string>, global::Kotlin.Pair>",
opt.GetOutputName ("System.Collections.Generic.IDictionary<System.Collections.Generic.IList<string>, Kotlin.Pair>"));

Assert.AreEqual ("global::System.Collections.Generic.IDictionary<global::System.Collections.Generic.IList<string>, global::System.Collections.Generic.IList<global::Kotlin.Pair>>",
Copy link
Member

Choose a reason for hiding this comment

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

These are all "positive" tests.

You should also test "negative" tests, "invalid" types. What happens if it attempts to parse System.Collections.Generic.List<> (missing type) or System.Collections.Generic.List<<> (too many <s) or System.Collections.Generic.List<>> (too many >s)?

Presumably it fails; does it? Does it fail in the way we expect? (Or does it instead return an empty string?)

Copy link
Member

@jonpryor jonpryor Sep 24, 2020

Choose a reason for hiding this comment

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

Should also try nested generic types, a'la

List<List<string>.Enumerator[]>.

opt.GetOutputName ("System.Collections.Generic.IDictionary<System.Collections.Generic.IList<string>, System.Collections.Generic.IList<Kotlin.Pair>>"));

Assert.AreEqual ("global::System.Collections.Generic.List<global::System.Collections.Generic.List<string>[]>[]",
opt.GetOutputName ("System.Collections.Generic.List<System.Collections.Generic.List<string>[]>[]"));

Assert.AreEqual ("global::System.Collections.Generic.List<global::System.Collections.Generic.List<string>.Enumerator[]>",
opt.GetOutputName ("System.Collections.Generic.List<System.Collections.Generic.List<string>.Enumerator[]>"));
}
}
}
42 changes: 10 additions & 32 deletions tools/generator/CodeGenerationOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -212,42 +212,20 @@ internal IEnumerable<string> GetJniMarshalDelegates ()
return jni_marshal_delegates;
}

public string GetOutputName (string s)
public string GetOutputName (string type)
{
if (s == "System.Void")
// Handle a few special cases
if (type == "System.Void")
return "void";
if (s.StartsWith ("params "))
return "params " + GetOutputName (s.Substring ("params ".Length));
if (s.StartsWith ("global::"))
if (type.StartsWith ("params "))
return "params " + GetOutputName (type.Substring ("params ".Length));
if (type.StartsWith ("global::"))
Report.LogCodedError (Report.ErrorUnexpectedGlobal);
if (!UseGlobal)
return s;
int idx = s.IndexOf ('<');
if (idx < 0) {
if (s.IndexOf ('.') < 0)
return s; // hack, to prevent things like global::int
return "global::" + s;
}
int idx2 = s.LastIndexOf ('>');
string sub = s.Substring (idx + 1, idx2 - idx - 1);
var typeParams = new List<string> ();
while (true) {
int idx3 = sub.IndexOf ('<');
int idx4 = sub.IndexOf (',');
if (idx4 < 0) {
typeParams.Add (GetOutputName (sub));
break;
} else if (idx3 < 0 || idx4 < idx3) { // more than one type params.
typeParams.Add (GetOutputName (sub.Substring (0, idx4)));
if (idx4 + 1 == sub.Length)
break;
sub = sub.Substring (idx4 + 1).Trim ();
} else {
typeParams.Add (GetOutputName (sub));
break;
}
}
return GetOutputName (s.Substring (0, idx)) + '<' + String.Join (", ", typeParams.ToArray ()) + '>';
return type;

// Add "global::" in front of types
return ParsedType.Parse (type).ToString (UseGlobal);
}

public string GetSafeIdentifier (string name)
Expand Down
103 changes: 103 additions & 0 deletions tools/generator/Utilities/ParsedType.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
using System.Collections.Generic;
using System.Linq;

namespace MonoDroid.Generation
{
// Parses a type string into its type and optionally its generic type arguments
// ex: Dictionary<string, List<string>>
// -> Type: Dictionary<{0}>
// - GenericArguments:
// - Type: string
// - Type: List<{0}>
// - GenericArguments:
// - Type: string
// A placeholder "{0}" is added because the type may extend past the generics:
// ex: "List<string>.Enumerator[]" becomes "List<{0}>.Enumerator[]"
public class ParsedType
{
public string Type { get; set; }
public List<ParsedType> GenericArguments { get; } = new List<ParsedType> ();
public bool HasGenerics => GenericArguments.Count > 0;

ParsedType () { }

public static ParsedType Parse (string type)
{
var less_than = type.IndexOf ('<');

// No generics
if (less_than < 0)
return new ParsedType { Type = type };

var greater_than = type.LastIndexOf ('>');
var type_args = type.Substring (less_than + 1, greater_than - less_than - 1);
var type_string = type.Substring (0, less_than) + "<{0}>" + (greater_than + 1 < type.Length ? type.Substring (greater_than + 1) : string.Empty);

var parsed_args = ParseTypeList (type_args);

var t = new ParsedType { Type = type_string };

foreach (var p in parsed_args)
t.GenericArguments.Add (Parse (p));

return t;
}

public override string ToString ()
{
return ToString (false);
}

public string ToString (bool useGlobal = false)
{
var type = (useGlobal && Type.IndexOf ('.') >= 0 ? "global::" : string.Empty) + Type;

if (!HasGenerics)
return type;

return type.Replace ("{0}", string.Join (", ", GenericArguments.Select (p => p.ToString (useGlobal))));
}

static List<string> ParseTypeList (string type)
{
var list = new List<string> ();

// Only one type
if (type.IndexOf (',') < 0) {
list.Add (type);
return list;
}

// Remove any whitespace
type = type.Replace (" ", "");

var start = 0;
var counter = -1;
var depth = 0;

while (++counter < type.Length) {
if (type [counter] == '<') {
depth++;
continue;
}

if (type [counter] == '>') {
depth--;
continue;
}

// This is a list separator, add the previous type
if (depth == 0 && type [counter] == ',') {
list.Add (type.Substring (start, counter - start));
start = counter + 1;
continue;
}
}

// Add the final type
list.Add (type.Substring (start, counter - start));

return list;
}
}
}