Skip to content

[generator] Gracefully handle BindingGeneratorException. #845

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 2 commits into from
May 26, 2021
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
26 changes: 13 additions & 13 deletions src/Java.Interop.Tools.Generator/Metadata/FixupXmlDocument.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ public void Apply (ApiXmlDocument apiDocument, string apiLevelString, int produc
else
// BG8A00
Report.LogCodedWarning (0, Report.WarningRemoveNodeMatchedNoNodes, null, metaitem, $"<remove-node path=\"{path}\" />");
} catch (XPathException e) {
} catch (XPathException) {
// BG4301
Report.LogCodedError (Report.ErrorRemoveNodeInvalidXPath, e, metaitem, path);
Report.LogCodedError (Report.ErrorRemoveNodeInvalidXPath, metaitem, path);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't we want to include e anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't use it anywhere, we simply print out the localized message.

}
break;
case "add-node":
Expand All @@ -74,9 +74,9 @@ public void Apply (ApiXmlDocument apiDocument, string apiLevelString, int produc
foreach (var node in nodes)
node.Add (metaitem.Nodes ());
}
} catch (XPathException e) {
} catch (XPathException) {
// BG4302
Report.LogCodedError (Report.ErrorAddNodeInvalidXPath, e, metaitem, path);
Report.LogCodedError (Report.ErrorAddNodeInvalidXPath, metaitem, path);
}
break;
case "change-node":
Expand All @@ -95,9 +95,9 @@ public void Apply (ApiXmlDocument apiDocument, string apiLevelString, int produc
if (!matched)
// BG8A03
Report.LogCodedWarning (0, Report.WarningChangeNodeTypeMatchedNoNodes, null, metaitem, $"<change-node-type path=\"{path}\" />");
} catch (XPathException e) {
} catch (XPathException) {
// BG4303
Report.LogCodedError (Report.ErrorChangeNodeInvalidXPath, e, metaitem, path);
Report.LogCodedError (Report.ErrorChangeNodeInvalidXPath, metaitem, path);
}
break;
case "attr":
Expand All @@ -106,7 +106,7 @@ public void Apply (ApiXmlDocument apiDocument, string apiLevelString, int produc

if (string.IsNullOrEmpty (attr_name))
// BG4307
Report.LogCodedError (Report.ErrorMissingAttrName, null, metaitem, path);
Report.LogCodedError (Report.ErrorMissingAttrName, metaitem, path);
var nodes = attr_last_cache != null ? new XElement [] { attr_last_cache } : apiDocument.ApiDocument.XPathSelectElements (path);
var attr_matched = 0;

Expand All @@ -119,9 +119,9 @@ public void Apply (ApiXmlDocument apiDocument, string apiLevelString, int produc
Report.LogCodedWarning (0, Report.WarningAttrMatchedNoNodes, null, metaitem, $"<attr path=\"{path}\" />");
if (attr_matched != 1)
attr_last_cache = null;
} catch (XPathException e) {
} catch (XPathException) {
// BG4304
Report.LogCodedError (Report.ErrorAttrInvalidXPath, e, metaitem, path);
Report.LogCodedError (Report.ErrorAttrInvalidXPath, metaitem, path);
}
break;
case "move-node":
Expand All @@ -140,9 +140,9 @@ public void Apply (ApiXmlDocument apiDocument, string apiLevelString, int produc
if (!matched)
// BG8A05
Report.LogCodedWarning (0, Report.WarningMoveNodeMatchedNoNodes, null, metaitem, $"<move-node path=\"{path}\" />");
} catch (XPathException e) {
} catch (XPathException) {
// BG4305
Report.LogCodedError (Report.ErrorMoveNodeInvalidXPath, e, metaitem, path);
Report.LogCodedError (Report.ErrorMoveNodeInvalidXPath, metaitem, path);
}
break;
case "remove-attr":
Expand All @@ -159,9 +159,9 @@ public void Apply (ApiXmlDocument apiDocument, string apiLevelString, int produc
if (!matched)
// BG8A06
Report.LogCodedWarning (0, Report.WarningRemoveAttrMatchedNoNodes, null, metaitem, $"<remove-attr path=\"{path}\" />");
} catch (XPathException e) {
} catch (XPathException) {
// BG4306
Report.LogCodedError (Report.ErrorRemoveAttrInvalidXPath, e, metaitem, path);
Report.LogCodedError (Report.ErrorRemoveAttrInvalidXPath, metaitem, path);
}
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public void ApplyFixupFile (FixupXmlDocument fixup)
fixup.Apply (this, ApiLevel, ProductVersion);
} catch (XmlException ex) {
// BG4200
Report.LogCodedError (Report.ErrorFailedToProcessMetadata, ex.Message);
Report.LogCodedErrorAndExit (Report.ErrorFailedToProcessMetadata, null, fixup.FixupDocument, ex.Message);
}
}
}
Expand Down
50 changes: 35 additions & 15 deletions src/Java.Interop.Tools.Generator/Utilities/Report.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,23 +69,30 @@ public LocalizedMessage (int code, string value)
public static LocalizedMessage WarningBaseInterfaceNotFound => new LocalizedMessage (0x8C00, Localization.Resources.Generator_BG8C00);
public static LocalizedMessage WarningBaseInterfaceInvalid => new LocalizedMessage (0x8C01, Localization.Resources.Generator_BG8C01);

public static void LogCodedError (LocalizedMessage message, params string [] args)
=> LogCodedError (message, null, null, -1, -1, args);
public static void LogCodedErrorAndExit (LocalizedMessage message, params string [] args)
=> LogCodedErrorAndExit (message, null, null, args);

public static void LogCodedError (LocalizedMessage message, Exception? innerException, params string [] args)
=> LogCodedError (message, innerException, null, -1, -1, args);
public static void LogCodedErrorAndExit (LocalizedMessage message, Exception? innerException, params string [] args)
=> LogCodedErrorAndExit (message, innerException, null, args);

public static void LogCodedError (LocalizedMessage message, Exception? innerException, XNode node, params string? [] args)
public static void LogCodedErrorAndExit (LocalizedMessage message, Exception? innerException, XNode? node, params string? [] args)
{
var file = Uri.TryCreate (node.BaseUri, UriKind.Absolute, out var uri) ? uri.LocalPath : null;
var line_info = (node as IXmlLineInfo)?.HasLineInfo () == true ? node as IXmlLineInfo : null;
LogCodedError (message, node, args);

LogCodedError (message, innerException, file, line_info?.LineNumber ?? -1, line_info?.LinePosition ?? -1, args);
// Throwing a BindingGeneratorException will cause generator to terminate
throw new BindingGeneratorException (message.Code, string.Format (message.Value, args), innerException);
}

public static void LogCodedError (LocalizedMessage message, Exception? innerException, string? sourceFile, int line, int column, params string? [] args)
public static void LogCodedError (LocalizedMessage message, XNode? node, params string? [] args)
{
throw new BindingGeneratorException (message.Code, sourceFile, line, column, string.Format (message.Value, args), innerException);
var (file, line, col) = GetLineInfo (node);

LogCodedError (message, file, line, col, args);
}

public static void LogCodedError (LocalizedMessage message, string? sourceFile, int line, int column, params string? [] args)
{
Console.Error.WriteLine (Format (true, message.Code, sourceFile, line, column, message.Value, args));
}

public static void LogCodedWarning (int verbosity, LocalizedMessage message, params string? [] args)
Expand All @@ -99,10 +106,9 @@ public static void LogCodedWarning (int verbosity, LocalizedMessage message, Exc

public static void LogCodedWarning (int verbosity, LocalizedMessage message, Exception? innerException, XNode node, params string? [] args)
{
var file = Uri.TryCreate (node.BaseUri, UriKind.Absolute, out var uri) ? uri.LocalPath : null;
var line_info = (node as IXmlLineInfo)?.HasLineInfo () == true ? node as IXmlLineInfo : null;
var (file, line, col) = GetLineInfo (node);

LogCodedWarning (verbosity, message, innerException, file, line_info?.LineNumber ?? -1, line_info?.LinePosition ?? -1, args);
LogCodedWarning (verbosity, message, innerException, file, line, col, args);
}

public static void LogCodedWarning (int verbosity, LocalizedMessage message, Exception? innerException, string? sourceFile, int line, int column, params string? [] args)
Expand Down Expand Up @@ -145,12 +151,26 @@ public static string Format (bool error, int errorCode, string? sourceFile, int
return ret + ": ";

if (column > 0)
return ret + $"({line}, {column}): ";
return ret + $"({line},{column}): ";

return ret + $"({line}): ";
}

static (string? file, int line, int col) GetLineInfo (XNode? node)
{
if (node is null)
return (null, -1, -1);

var file = Uri.TryCreate (node.BaseUri, UriKind.Absolute, out var uri) ? uri.LocalPath : null;
var pos = (node as IXmlLineInfo)?.HasLineInfo () == true ? node as IXmlLineInfo : null;

return (file, pos?.LineNumber ?? -1, pos?.LinePosition ?? -1);
}
}


/// <summary>
/// Throwing this exception will cause generator to exit gracefully.
/// </summary>
public class BindingGeneratorException : Exception
{
public BindingGeneratorException (int errorCode, string message)
Expand Down
4 changes: 2 additions & 2 deletions tests/generator-Tests/Unit-Tests/ReportTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ public void FormatTests ()
Assert.AreEqual ("error BG0037: There was a bad error", Report.Format (true, code, null, 0, 0, msg, args));
Assert.AreEqual (@"C:\code\test.cs: error BG0037: There was a bad error", Report.Format (true, code, sourcefile, 0, 0, msg, args));
Assert.AreEqual (@"C:\code\test.cs(32): error BG0037: There was a bad error", Report.Format (true, code, sourcefile, line, 0, msg, args));
Assert.AreEqual (@"C:\code\test.cs(32, 12): error BG0037: There was a bad error", Report.Format (true, code, sourcefile, line, col, msg, args));
Assert.AreEqual (@"C:\code\test.cs(32, 12): warning BG0037: There was a bad error", Report.Format (false, code, sourcefile, line, col, msg, args));
Assert.AreEqual (@"C:\code\test.cs(32,12): error BG0037: There was a bad error", Report.Format (true, code, sourcefile, line, col, msg, args));
Assert.AreEqual (@"C:\code\test.cs(32,12): warning BG0037: There was a bad error", Report.Format (false, code, sourcefile, line, col, msg, args));
}
}
}
2 changes: 1 addition & 1 deletion tools/generator/CodeGenerationOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ public string GetOutputName (string type)
if (type.StartsWith ("params "))
return "params " + GetOutputName (type.Substring ("params ".Length));
if (type.StartsWith ("global::"))
Report.LogCodedError (Report.ErrorUnexpectedGlobal);
Report.LogCodedErrorAndExit (Report.ErrorUnexpectedGlobal);
if (!UseGlobal)
return type;

Expand Down
14 changes: 11 additions & 3 deletions tools/generator/CodeGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,19 @@ public class CodeGenerator
{
public static int Main (string[] args)
{
var options = CodeGeneratorOptions.Parse (args);
if (options == null)
try {
var options = CodeGeneratorOptions.Parse (args);
if (options == null)
return 1;

Run (options);
} catch (BindingGeneratorException) {
return 1;
} catch (Exception ex) {
Console.Error.WriteLine (Report.Format (true, 0, null, -1, -1, ex.ToString ()));
return 1;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should other exception types be handled, "just in case", so that we don't get the "unhandled exception" output? Or do we want the unhandled exception output?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, added.


Run (options);
return 0;
}

Expand Down
Loading