CSharpCodeCompiler should treat multi-line warning messages as warnings #2248
Conversation
|
Hey @dmitryvk, However, it looks like you haven't signed our CLA (Contributor License Agreement) yet. In order for us to accept your pull request, you have to sign our CLA first. You can read and sign our full Contributor License Agreement here. Thanks, Your friendly Xamarin CLA Bot# |
|
Hello! I'm the build bot for the Mono project. I need approval from a Mono team member to build this pull request. A team member should reply with "approve" to approve a build of this pull request, "whitelist" to whitelist this and all future pull requests from this contributor, or "build" to explicitly request a build, even if one has already been done. Contributors can ignore this message. |
|
Can you add a unit test please? E.g. here: https://github.com/mono/mono/blob/master/mcs/class/System/Test/Microsoft.CSharp/CSharpCodeProviderTest.cs |
|
Hey @dmitryvk, Thanks for signing our CLA! We can now look at your pull request. Always at your service, Your friendly Xamarin CLA Bot# |
…nternal compiler errors. Most errors and warnings from c# compiler are reported in one line in format "FILE(LINE, COL): warning CS1234: Text". But some warning span multiple lines: /tmp/3fb23cc0/634d2022.0.cs(22,29): warning CS0114: `NS.B.F()' hides inherited member `NS.A.F()' .. /tmp/3fb23cc0/634d2022.0.cs(16,29): (Location of the symbol related to previous warning) Previously the second line was considered to be an internal compiler error since it doesn't match the error regex. This seems to be only type of messages that span multiple lines. This fixes #35980
48c52b3 to
0a5ff97
Compare
|
Added the unit test. |
|
@monojenkins build |
|
Looks good to me, thanks. |
CSharpCodeCompiler should treat multi-line warning messages as warnings
The internal Xamarin.Android CI system is reporting an error when running the `generator` unit tests, but it's a false positive. It's *not* a bug in `generator`. Rather, it's a bug in Mono 4.2, in which [`CSharpCodeCompiler` treats multi-line warnings as errors][0]. Since the `generator` unit tests emit such multi-line warnings when hiding `Java.Lang.Object.Handle` (via `Xamarin.Test.SomeObject.Handle()`) and `System.Exception.Message` (via `Java.Lang.Throwable.Message`), and Mono 4.2 treats the multi-line warnings as errors, the tests fail. There are three plausible solutions so that we don't get erroneous reports from the `generator` tests when running on Mono 4.2: 1. Remove those tests. (Uh...no.) 2. Upgrade Mono on the CI machine to Mono 4.4, which has the fix. 3. Fix `generator` so that the warning isn't emitted. (2) would require an unknown timeframe with unknown repercussions. Thus, the chosen fix: improve `generator` so that the warning isn't generated. [0]: mono/mono#2248
The internal Xamarin.Android CI system is reporting an error when running the `generator` unit tests, but it's a false positive. It's *not* a bug in `generator`. Rather, it's a bug in Mono 4.2, in which [`CSharpCodeCompiler` treats multi-line warnings as errors][0]. Since the `generator` unit tests emit such multi-line warnings when hiding `Java.Lang.Object.Handle` (via `Xamarin.Test.SomeObject.Handle()`) and `System.Exception.Message` (via `Java.Lang.Throwable.Message`), and Mono 4.2 treats the multi-line warnings as errors, the tests fail. There are three plausible solutions so that we don't get erroneous reports from the `generator` tests when running on Mono 4.2: 1. Remove those tests. (Uh...no.) 2. Upgrade Mono on the CI machine to Mono 4.4, which has the fix. 3. Fix `generator` so that the warning isn't emitted. (2) would require an unknown timeframe with unknown repercussions. Thus, the chosen fix: improve `generator` so that the warning isn't generated. [0]: mono/mono#2248
Most errors and warnings from c# compiler are reported in one line in format "FILE(LINE, COL): warning CS1234: Text".
But some warning span multiple lines:
Previously the second line was considered to be an internal compiler error since it doesn't match the error regex.
This seems to be only type of messages that span multiple lines.
This fixes #35980