Warning instead of error

[Pzixel]

I currently have my generator fail for some known reasons. But instead of showing a nice error it just silently throws a warning, and then some depended code fails. It doesn't seem right:

1>C:\Users\Alex\Documents\Repo\fairs-ethereum\OrbitaCenter.Fairs.Solidity\Contract.sol(435,49): warning G8C7AD368: Undeclared identifier.
1>C:\Users\Alex\.nuget\packages\codegeneration.roslyn.buildtime\0.4.74\build\CodeGeneration.Roslyn.BuildTime.targets(42,5): warning MSB3073: The command "dotnet codegen "@obj\Debug\netstandard2.0\OrbitaCenter.Fairs.Solidity.csproj.dotnet-codegen.rsp"" exited with code 3.
1>C:\Users\Alex\.nuget\packages\codegeneration.roslyn.buildtime\0.4.74\build\CodeGeneration.Roslyn.BuildTime.targets(44,5): warning CGR1000: dotnet-codegen: Failed to generate the list of generated files. The tool didn't run succesfully. Please check https://github.com/AArnott/CodeGeneration.Roslyn for usage instructions.
1>    3 Warning(s)
1>    0 Error(s)
1>
1>Time Elapsed 00:00:03.01
========== Rebuild All: 1 succeeded, 0 failed, 0 skipped ==========

[amis92]

Can you tell what is that known error?

I've updated the build task recently, but I think I kept the silent warning behavior. I may be wrong, or we need to change the behavior.

[Pzixel]

I'm talking about this project: https://github.com/Pzixel/Solidity.Roslyn

It generates C# wrappers for solidity types so they are callable from C#. It's a known issue that user may have invalid solidity souce file (I missed an identifier in the example above). In this case solidity compiler throws an error and it's clear that I cannot generate C# wrappers, because I should get AST from solidity compiler, but instead I get bunch of errors.

In this case I just want to stop the compilation process and show errors to the user for he could fix them.

[amis92]

I might've introduced that behavior. If so, it originates in https://github.com/AArnott/CodeGeneration.Roslyn/pull/83/files#diff-94e71052ac3ede7cfac37443a34398a9

I'm not sure now if the pre-change behavior also resulted in warnings. It's possible it returned errors. I'll investigate and push a PR with a fix, but it might take me some time.

The fix most probably requires removal of ContinueOnError attribute at https://github.com/AArnott/CodeGeneration.Roslyn/pull/83/files#diff-94e71052ac3ede7cfac37443a34398a9R38

---

I've browsed through error handling in the tool. Currently it's a chaotic mashup of direct-console logging, IProgress reporting and a custom Logger class. I do believe that a refactor is in order.

I think that IProgress should stay, as designed, an output channel for generators. I am not sure what the role of a custom Logger is, but it is a public interface that allows a much simpler output, directly to console but with the Console hidden within implementation.

Then there are instances of direct Console usage, like:

• Program.cs error handling (and it seems it was me who changed it from Logger to Console - I think it was because the original behavior caused a lot of duplicated error reporting, and I tried to de-duplicate it at least in Visual Studio Error List).
  

  CodeGeneration.Roslyn/src/CodeGeneration.Roslyn.Tool/Program.cs

  Lines 38 to 42 in 38605a4

  	if (!compile.Any())
  	{
  	Console.Error.WriteLine("No source files are specified.");
  	return 1;
  	}

  
  

  CodeGeneration.Roslyn/src/CodeGeneration.Roslyn.Tool/Program.cs

  Lines 44 to 48 in 38605a4

  	if (outputDirectory == null)
  	{
  	Console.Error.WriteLine("The output directory must be specified.");
  	return 2;
  	}

  
  

  CodeGeneration.Roslyn/src/CodeGeneration.Roslyn.Tool/Program.cs

  Lines 66 to 71 in 38605a4

  	catch (Exception e)
  	{
  	Console.Error.WriteLine($"{e.GetType().Name}: {e.Message}");
  	Console.Error.WriteLine(e.ToString());
  	return 3;
  	}

  CodeGeneration.Roslyn/src/CodeGeneration.Roslyn.Tool/Program.cs

  Lines 86 to 89 in 38605a4

  	private static void OnDiagnosticProgress(Diagnostic diagnostic)
  	{
  	Console.WriteLine(diagnostic.ToString());
  	}

• In CompilationGenerator.cs there is an additional log of the error into a Console.
  

  CodeGeneration.Roslyn/src/CodeGeneration.Roslyn/CompilationGenerator.cs

  Line 245 in 38605a4

  Console.Error.WriteLine($"Exception in file processing: {ex}");

• And of course in the Logger:
  

  CodeGeneration.Roslyn/src/CodeGeneration.Roslyn/Logger.cs

  Lines 59 to 70 in 38605a4

  	void Print(string toPrint)
  	{
  	if (logLevel == LogLevel.Info)
  	{
  	Console.WriteLine(toPrint);
  	}
  	else
  	{
  	// log using MSBuild convention of [Origin:] [Subcategory] Category Code: [Text]
  	Console.WriteLine($"dotnet-codegen: {logLevel} {diagnosticCode}: {toPrint}");
  	}
  	}

[LokiMidgard]

I think the logger was initial created by me. The previous console output was not shown in the build output with default verbosity. If I remembered correctly.

[Pzixel]

IIRC IProgress is always null since it's never gets assigned. I could be wrong, but when I tried to use it I was always gettin NRE.

---

https://github.com/Pzixel/RemoteClient.Roslyn/blob/master/RemoteClient.Roslyn/RemoteClientGenerator.cs#L34 - as you can see, I use null propagation becaus IProgress may be missing. It's really not convinient, but I didn't come to the better solution at the time.

Another point is that OnDiagnosticProgress just prints the diagnostics where it probably should do something like

private static void OnDiagnosticProgress(Diagnostic diagnostic)
{
	if (diagnostic.Severity == DiagnosticSeverity.Error)
	{
		Console.Error.WriteLine(diagnostic.ToString());
	}
	Console.WriteLine(diagnostic.ToString());
}

[amis92]

IProgress was fixed with PR #45 and is now working:

CodeGeneration.Roslyn/src/CodeGeneration.Roslyn.Tool/Program.cs

Line 60 in 38605a4

var progress = new Progress<Diagnostic>(OnDiagnosticProgress);

It might be a good idea to print the diagnostic on error output for error level.

[AArnott]

I think the inconsistency is the result of the evolution of the generator initially being T4, then an MSBuild task, and now a dotnet CLI tool.
Ideally, when run in an MSBuild invocation, the errors/warnings are reported via the MSBuild reporting mechanism for such. But I don't know how we can properly pipe that through to the task. But MSBuild has excellent built-in parsing support for invoked tools, so long as they follow a standard format for warning/error output messages. Can we make sure that we follow that pattern, and then adjust the msbuild target that invokes our tool to set the option that MSBuild should parse the output, looking for errors/warnings?

[amis92]

I think the Logger already does that here:

CodeGeneration.Roslyn/src/CodeGeneration.Roslyn/Logger.cs

Lines 59 to 70 in 38605a4

	void Print(string toPrint)
	{
	if (logLevel == LogLevel.Info)
	{
	Console.WriteLine(toPrint);
	}
	else
	{
	// log using MSBuild convention of [Origin:] [Subcategory] Category Code: [Text]
	Console.WriteLine($"dotnet-codegen: {logLevel} {diagnosticCode}: {toPrint}");
	}
	}

The only problem is that it will parse a new error/warning for each line of the message logged.

Also, isn't that "parsing" the default behavior of MSBuild?

[AArnott]

Yes, that may all be set up. I was just speaking from memory and speculating based on what I'm seeing here.

[amis92]

Marking as bug because I believe there was a change from the failure to generate being an error to a warning. It should be probably reverted, because using generators should be conscious, and current behavior doesn't work well for users.

[Pzixel]

Any progress on that?