Skip to content

Commit

Permalink
[generator] Fix MSBuild warning/error format for Visual Studio (#765)
Browse files Browse the repository at this point in the history
In b858dc5 we updated `generator` warnings/errors to give line &
column information in more places.  However, the existing method for
formatting the line & column information was wrong:

	// Correct
	C:\code\Metadata.xml(2, 6): warning BG8A04: <attr path="/api/package[@name='androidx.appcompat.widget']/class[@name='RoundRectDrawableWithShadow']"/> matched no nodes.

	// Incorrect
	C:\code\Metadata.xml(2, 6) warning BG8A04: <attr path="/api/package[@name='androidx.appcompat.widget']/class[@name='RoundRectDrawableWithShadow']"/> matched no nodes.

By omitting the colon after the line & column information,
Visual Studio parses the colon within `C:\` instead, resulting in:

![image](https://user-images.githubusercontent.com/179295/102400384-7500b100-3fa7-11eb-8f35-aae2a04f3a58.png)

  * Filename: `C`
  * Line number: 1

This is actually worse than what we previously had, as double-clicking
it does nothing, as `C` is not a valid file on disk.

Rewrite the `Report.Format()` method to be a little clearer to read
and add the required colon.
  • Loading branch information
jpobst authored Dec 17, 2020
1 parent 3f6cf72 commit 876442f
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 3 deletions.
30 changes: 30 additions & 0 deletions tests/generator-Tests/Unit-Tests/ReportTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using MonoDroid.Generation;
using NUnit.Framework;

namespace generatortests
{
public class ReportTests
{
[Test]
public void FormatTests ()
{
var code = 0x37;
var msg = "There was a {0} error";
var args = "bad";
var sourcefile = @"C:\code\test.cs";
var line = 32;
var col = 12;

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));
}
}
}
22 changes: 19 additions & 3 deletions tools/generator/Utilities/Report.cs
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,25 @@ public static string FormatCodedMessage (bool error, LocalizedMessage message, p

public static string Format (bool error, int errorCode, string sourceFile, int line, int column, string format, params object[] args)
{
var origin = sourceFile != null ? sourceFile + (line > 0 ? column > 0 ? $"({line}, {column})" : $"({line})" : null) + ' ' : null;
return string.Format ("{0}{1} BG{2:X04}: ", origin, error ? "error" : "warning", errorCode) +
string.Format (format, args);
var origin = FormatOrigin (sourceFile, line, column);

return $"{origin}{(error ? "error" : "warning")} BG{errorCode:X04}: " + string.Format (format, args);
}

static string FormatOrigin (string sourceFile, int line, int column)
{
if (string.IsNullOrWhiteSpace (sourceFile))
return null;

var ret = sourceFile;

if (line == 0)
return ret + ": ";

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

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

Expand Down

0 comments on commit 876442f

Please sign in to comment.