Skip to content
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

Generated files (e.g, Java.Interop.__TypeRegistrations.cs) should mark themselves as auto-generated #1051

Closed
Youssef1313 opened this issue Oct 15, 2022 · 3 comments · Fixed by #1052
Assignees
Labels
enhancement Proposed change to current functionality generator Issues binding a Java library (generator, class-parse, etc.)
Milestone

Comments

@Youssef1313
Copy link
Member

Android application type

Classic Xamarin.Android (MonoAndroid12.0, etc.), Android for .NET (net6.0-android, etc.)

Affected platform version

MonoAndroid13.0 and net6.0-android

Description

With a project where CA1812 and CA are enforced as errors, the following is observed:

##[error]src\Uno.UI.BindingHelper.Android\obj\Release\net6.0-android\generated\src\Java.Interop.__TypeRegistrations.cs(16,6): Error CA1825: Avoid unnecessary zero-length array allocations.  Use Array.Empty<string>() instead.
C:\a\1\s\src\Uno.UI.BindingHelper.Android\obj\Release\net6.0-android\generated\src\Java.Interop.__TypeRegistrations.cs(16,6): error CA1825: Avoid unnecessary zero-length array allocations.  Use Array.Empty<string>() instead. [C:\a\1\s\src\Uno.UI.BindingHelper.Android\Uno.UI.BindingHelper.Android.net6.csproj]
##[error]src\Uno.UI.BindingHelper.Android\obj\Release\net6.0-android\generated\src\Java.Interop.__TypeRegistrations.cs(18,6): Error CA1825: Avoid unnecessary zero-length array allocations.  Use Array.Empty<Converter<string, Type>>() instead.
C:\a\1\s\src\Uno.UI.BindingHelper.Android\obj\Release\net6.0-android\generated\src\Java.Interop.__TypeRegistrations.cs(18,6): error CA1825: Avoid unnecessary zero-length array allocations.  Use Array.Empty<Converter<string, Type>>() instead. [C:\a\1\s\src\Uno.UI.BindingHelper.Android\Uno.UI.BindingHelper.Android.net6.csproj]
##[error]src\Uno.UI.BindingHelper.Android\obj\Release\net6.0-android\generated\src\Uno.UI.UnoRecyclerView.cs(168,25): Error CA1812: 'UnoRecyclerViewInvoker' is an internal class that is apparently never instantiated. If so, remove the code from the assembly. If this class is intended to contain only static members, make it 'static' (Module in Visual Basic).
C:\a\1\s\src\Uno.UI.BindingHelper.Android\obj\Release\net6.0-android\generated\src\Uno.UI.UnoRecyclerView.cs(168,25): error CA1812: 'UnoRecyclerViewInvoker' is an internal class that is apparently never instantiated. If so, remove the code from the assembly. If this class is intended to contain only static members, make it 'static' (Module in Visual Basic). [C:\a\1\s\src\Uno.UI.BindingHelper.Android\Uno.UI.BindingHelper.Android.net6.csproj]
##[error]src\Uno.UI.BindingHelper.Android\obj\Release\net6.0-android\generated\src\Uno.UI.UnoViewGroup.cs(829,25): Error CA1812: 'UnoViewGroupInvoker' is an internal class that is apparently never instantiated. If so, remove the code from the assembly. If this class is intended to contain only static members, make it 'static' (Module in Visual Basic).
C:\a\1\s\src\Uno.UI.BindingHelper.Android\obj\Release\net6.0-android\generated\src\Uno.UI.UnoViewGroup.cs(829,25): error CA1812: 'UnoViewGroupInvoker' is an internal class that is apparently never instantiated. If so, remove the code from the assembly. If this class is intended to contain only static members, make it 'static' (Module in Visual Basic). [C:\a\1\s\src\Uno.UI.BindingHelper.Android\Uno.UI.BindingHelper.Android.net6.csproj]
  1. These files should have // <auto-generated/> comment at the top so that Roslyn marks them as auto-generated and skips most analyzers for them.
  2. The above violations should be fixed if possible.

Steps to Reproduce

Did you find any workaround?

No response

Relevant log output

No response

@jpobst jpobst transferred this issue from dotnet/android Oct 15, 2022
@jpobst jpobst added this to the 8.0.0 milestone Oct 15, 2022
@jpobst jpobst added enhancement Proposed change to current functionality generator Issues binding a Java library (generator, class-parse, etc.) and removed needs-triage labels Oct 15, 2022
@jpobst
Copy link
Contributor

jpobst commented Oct 18, 2022

Some examples of what a regular console .NET 6 program generates:

  • .NETCoreApp,Version=v6.0.AssemblyAttributes.cs:
// <autogenerated />
using System;
...
  • {application}.AssemblyInfo.cs:
//------------------------------------------------------------------------------
// <auto-generated>
//     This code was generated by a tool.
//     Runtime Version:4.0.30319.42000
//
//     Changes to this file may cause incorrect behavior and will be lost if
//     the code is regenerated.
// </auto-generated>
//------------------------------------------------------------------------------
using System;
...

Using the longer version could also help with issues like dotnet/android#7449.

@jpobst
Copy link
Contributor

jpobst commented Oct 18, 2022

Can you post a Java.Interop.__TypeRegistrations.cs that demonstrates the CA1825 warning? It looks like all the arrays should have content in them, unless your binding library doesn't bind any types or something? Or maybe all the types it binds are static?

Example:

		public static void RegisterPackages ()
		{
#if MONODROID_TIMING
			var start = DateTime.Now;
			Android.Util.Log.Info ("MonoDroid-Timing", "RegisterPackages start: " + (start - new DateTime (1970, 1, 1)).TotalMilliseconds);
#endif // def MONODROID_TIMING
			Java.Interop.TypeManager.RegisterPackages (
					new string[]{
						"com/google/common/flogger",
						"com/google/common/flogger/backend",
						"com/google/common/flogger/context",
						"com/google/common/flogger/parameter",
						"com/google/common/flogger/parser",
						"com/google/common/flogger/util",
					},
					new Converter<string, Type?>[]{
						lookup_com_google_common_flogger_package,
						lookup_com_google_common_flogger_backend_package,
						lookup_com_google_common_flogger_context_package,
						lookup_com_google_common_flogger_parameter_package,
						lookup_com_google_common_flogger_parser_package,
						lookup_com_google_common_flogger_util_package,
					});
#if MONODROID_TIMING
			var end = DateTime.Now;
			Android.Util.Log.Info ("MonoDroid-Timing", "RegisterPackages time: " + (end - new DateTime (1970, 1, 1)).TotalMilliseconds + " [elapsed: " + (end - start).TotalMilliseconds + " ms]");
#endif // def MONODROID_TIMING
		}

@Youssef1313
Copy link
Member Author

@jpobst This is what I'm seeing:

using System;
using System.Collections.Generic;
using Android.Runtime;

namespace Java.Interop {

	partial class __TypeRegistrations {

		public static void RegisterPackages ()
		{
#if MONODROID_TIMING
			var start = DateTime.Now;
			Android.Util.Log.Info ("MonoDroid-Timing", "RegisterPackages start: " + (start - new DateTime (1970, 1, 1)).TotalMilliseconds);
#endif // def MONODROID_TIMING
			Java.Interop.TypeManager.RegisterPackages (
					new string[]{
					},
					new Converter<string, Type>[]{
					});
#if MONODROID_TIMING
			var end = DateTime.Now;
			Android.Util.Log.Info ("MonoDroid-Timing", "RegisterPackages time: " + (end - new DateTime (1970, 1, 1)).TotalMilliseconds + " [elapsed: " + (end - start).TotalMilliseconds + " ms]");
#endif // def MONODROID_TIMING
		}

#if NET5_0_OR_GREATER
		[System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessage ("Trimming", "IL2057")]
#endif
		static Type Lookup (string[] mappings, string javaType)
		{
			var managedType = Java.Interop.TypeManager.LookupTypeMapping (mappings, javaType);
			if (managedType == null)
				return null;
			return Type.GetType (managedType);
		}
	}
}

jonpryor pushed a commit that referenced this issue Oct 20, 2022
Fixes:  #1051

Context: dotnet/android#7449

[Roslyn determines if a file is generated or not][0] by looking for
the strings `<autogenerated` or `<auto-generated` within the file.

Roslyn analyzers specify whether they want to scan generated code or
not using [`GeneratedCodeAnalysisFlags`][1].  Many analyzers *disable*
their checks when processing generated code.

Until now, `generator` output did *not* indicate that the output is
generated code.  Consequently, many Roslyn analyzers would process
and issue warnings about generated code, warnings which would *not*
be emitted if the analyzer knew it was generated code:

	…\src\Uno.UI.BindingHelper.Android\obj\Release\net6.0-android\generated\src\Java.Interop.__TypeRegistrations.cs(16,6): error CA1825: Avoid unnecessary zero-length array allocations.  Use Array.Empty<string>() instead.
	…\src\Uno.UI.BindingHelper.Android\obj\Release\net6.0-android\generated\src\Java.Interop.__TypeRegistrations.cs(18,6): error CA1825: Avoid unnecessary zero-length array allocations.  Use Array.Empty<Converter<string, Type>>() instead.
	…\src\Uno.UI.BindingHelper.Android\obj\Release\net6.0-android\generated\src\Uno.UI.UnoRecyclerView.cs(168,25): error CA1812: 'UnoRecyclerViewInvoker' is an internal class that is apparently never instantiated. If so, remove the code from the assembly. If this class is intended to contain only static members, make it 'static' (Module in Visual Basic).
	…\src\Uno.UI.BindingHelper.Android\obj\Release\net6.0-android\generated\src\Uno.UI.UnoViewGroup.cs(829,25): error CA1812: 'UnoViewGroupInvoker' is an internal class that is apparently never instantiated. If so, remove the code from the assembly. If this class is intended to contain only static members, make it 'static' (Module in Visual Basic).

Update `generator` so that generated `.cs` files contain the comment:

	//------------------------------------------------------------------------------
	// <auto-generated>
	//     This code was generated by a tool.
	//
	//     Changes to this file may cause incorrect behavior and will be lost if
	//     the code is regenerated.
	// </auto-generated>
	//------------------------------------------------------------------------------

This will help ensure that Roslyn considers the files to contain
generated code, which will prevent some analyzers from checking those
files, which will reduce the number of warnings or errors emitted.

This should allow slightly faster builds with fewer warnings.
Additionally, it should help make it clearer to users that these
files should not be modified by hand (dotnet/android#7449).

Note: by default [Roslyn disables Nullable Reference Types (NRT)][2]
for code marked as "generated", causing:

	Error CS8669: The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.
	Auto-generated code requires an explicit '#nullable' directive in source.

We have to explicitly restore NRT back to the project settings for
our generated files with:

	#nullable restore

[0]: https://github.com/dotnet/roslyn/blob/3f694a3c39c44b5dccb4aaaa163080bdb3ad412f/src/Compilers/Core/Portable/InternalUtilities/GeneratedCodeUtilities.cs
[1]: https://learn.microsoft.com/en-us/dotnet/api/microsoft.codeanalysis.diagnostics.analysiscontext.configuregeneratedcodeanalysis
[2]: https://github.com/dotnet/roslyn/blob/70e158ba6c2c99bd3c3fc0754af0dbf82a6d353d/docs/features/nullable-reference-types.md#generated-code
jonpryor pushed a commit to dotnet/android that referenced this issue Nov 1, 2022
Fixes: dotnet/java-interop#1034
Fixes: dotnet/java-interop#1051

Context: 938b2cb

Changes: dotnet/java-interop@e1ee4b1...5318261

  * dotnet/java-interop@53182615: [build] Update Microsoft.* NuGet package versions (dotnet/java-interop#1055)
  * dotnet/java-interop@8e18c909: [generator] Avoid C#11 delegate cache overhead. (dotnet/java-interop#1053)
  * dotnet/java-interop@2d8b6d24: [generator] More AttributeTargets on SupportedOSPlatformAttribute (dotnet/java-interop#1054)
  * dotnet/java-interop@7dfbab67: [generator] Add [SupportedOSPlatform] to bound constant fields (dotnet/java-interop#1038)
  * dotnet/java-interop@1720628a: [generator] Mark generated .cs files as generated (dotnet/java-interop#1052)
  * dotnet/java-interop@f498fcf5: [Java.Interop] Avoid some method group conversions (dotnet/java-interop#1050)
  * dotnet/java-interop@16e1ecd4: [build] Use $(VSINSTALLDIR), not $(VSINSTALLROOT) (dotnet/java-interop#1048)
  * dotnet/java-interop@8e4c7d20: [Hello-Core] Add "low level" sample. (dotnet/java-interop#1047)

Additionally, remove `$(LangVersion)`=10 from `Mono.Android.csproj`,
which was a hack to work around a size regression due to delegate
caching in C# 11; see also 938b2cb, dotnet/java-interop@8e18c909.

Co-authored-by: Jonathan Pobst <[email protected]>
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Proposed change to current functionality generator Issues binding a Java library (generator, class-parse, etc.)
Projects
None yet
2 participants