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

LayoutBindings: 'PreserveAttribute' is obsolete warnings #7480

Closed
rgroenewoudt opened this issue Oct 21, 2022 · 13 comments · Fixed by #8529
Closed

LayoutBindings: 'PreserveAttribute' is obsolete warnings #7480

rgroenewoudt opened this issue Oct 21, 2022 · 13 comments · Fixed by #8529
Assignees
Labels
Area: App+Library Build Issues when building Library projects or Application projects.

Comments

@rgroenewoudt
Copy link

Android application type

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

Affected platform version

VS2022 17.3.6

Description

We are using a lot of AndroidBoundLayout in our app but this results in 2 warnings per layout file:

warning CS0618: 'PreserveAttribute' is obsolete: 'Please use [System.Diagnostics.CodeAnalysis.DynamicDependencyAttribute]'
[global::Android.Runtime.PreserveAttribute (Conditional=true)]
public AboutSequriX (
	global::Android.App.Activity client,
	global::Xamarin.Android.Design.OnLayoutItemNotFoundHandler itemNotFoundHandler = null)
		: base (client, itemNotFoundHandler)
{}

[global::Android.Runtime.PreserveAttribute (Conditional=true)]
public AboutSequriX (
	global::Android.Views.View client,
	global::Xamarin.Android.Design.OnLayoutItemNotFoundHandler itemNotFoundHandler = null)
		: base (client, itemNotFoundHandler)
{}

We would like to get to 0 build warnings :)

Steps to Reproduce

Did you find any workaround?

No response

Relevant log output

No response

@rgroenewoudt rgroenewoudt added Area: App+Library Build Issues when building Library projects or Application projects. needs-triage Issues that need to be assigned. labels Oct 21, 2022
@dellis1972
Copy link
Contributor

Those look like they are coming from the Xamarin.Android.Support.Design Nuget Package.
I believe the recomendation now is to upgrade to use AndroidX rather than the old packages.
So you will need to migrate the code away from the old Support libraries to AndroidX.

See https://learn.microsoft.com/en-us/xamarin/android/platform/androidx for more details.

The PreserveAttribute is going to be removed in a future version of .net so migrating is the best option.

@rgroenewoudt
Copy link
Author

@dellis1972 I don't have any nuget packages Xamarin.Android.Support or any referencing it. Only AndroidX.

Example project. I created a new .NET 6 Android project, changed activity_main.xml to AndroidBoundlayout and added an ID.
AndroidApp2.zip

1>C:\git\AndroidApp2\AndroidApp2\obj\Debug\net6.0-android\generated\Binding.activity_main.g.cs(12,4,12,45): warning CS0618: 'PreserveAttribute' is obsolete: 'Please use [System.Diagnostics.CodeAnalysis.DynamicDependencyAttribute]'
1>C:\git\AndroidApp2\AndroidApp2\obj\Debug\net6.0-android\generated\Binding.activity_main.g.cs(19,4,19,45): warning CS0618: 'PreserveAttribute' is obsolete: 'Please use [System.Diagnostics.CodeAnalysis.DynamicDependencyAttribute]'

@dellis1972
Copy link
Contributor

Ah. Are you using LayoutBindings?

@dellis1972
Copy link
Contributor

Ignore that, yes you are :) sorry.

OK, that is where the issue is then.

@dellis1972 dellis1972 removed the needs-triage Issues that need to be assigned. label Oct 21, 2022
@jonathanpeppers jonathanpeppers added this to the Under Consideration milestone Oct 21, 2022
@jonathanpeppers jonathanpeppers changed the title 'PreserveAttribute' is obsolete warnings LayoutBindings: 'PreserveAttribute' is obsolete warnings Oct 21, 2022
@rgroenewoudt
Copy link
Author

I would guess that there would need to be an if added to support both classic with PreserveAttribute and .NET 6 with DynamicDependencyAttribute.
https://github.com/xamarin/xamarin-android/blob/ab6712a48e4b93e9c3d605c7b393814d477e2bc8/src/Xamarin.Android.Build.Tasks/Tasks/GenerateLayoutBindings.CSharpBindingGenerator.cs#L221

A additional property would be probably be needed to feed the MsBuild task, perhaps UsingAndroidNETSdk?
https://github.com/xamarin/xamarin-android/blob/ab6712a48e4b93e9c3d605c7b393814d477e2bc8/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets#L1152

@dellis1972
Copy link
Contributor

DynamicDependencyAttribute works in the reverse manner to the old PreserveAttribute. The old system you placed the attribute at the code you wanted to preserver. The new DynamicDependencyAttribute you have to place it where the type is consumed. So we need to figure out the best way to deal with this without requiring the user to add the DynamicDependencyAttribute to the activity where its used.

@rgroenewoudt
Copy link
Author

Is the attribute even needed? This is the C# binding which is used by the developer so if unused constructors are removed it shouldn't be a problem?

The file should also contain the autogenerated comment so any warnings would be ignored.

@jpobst
Copy link
Contributor

jpobst commented Nov 28, 2022

@rgroenewoudt
Copy link
Author

I'm still seeing this issue in NET 7 Android 33, even though #1051 is resolved.

There is no autogenerated comment in generated/Binding.xxx.g.cs

@jpobst
Copy link
Contributor

jpobst commented Mar 13, 2023

This change was too late to make it into .NET 7, I would suspect that it is fixed in .NET 8 Previews.

@rgroenewoudt
Copy link
Author

Same issue in .NET 8.
xxxx\obj\Debug\net8.0-android\codebehind\Binding.StartWorkShift.g.cs(19,4,19,45): warning CS0618: 'PreserveAttribute' is obsolete: 'Please use [System.Diagnostics.CodeAnalysis.DynamicDependencyAttribute]'

dellis1972 added a commit to dellis1972/xamarin-android that referenced this issue Nov 30, 2023
…ings

Fixes dotnet#7480

* Remove the use of `PreserveAttribute` as we no longer support Xamarin
  Classic in main.
* Add #pragma and code comments to fix certain CS* warnings which are
  emitted by the C# compiler.
* Ignore a bunch of other warnings in the unit test.
@rgroenewoudt
Copy link
Author

rgroenewoudt commented Dec 21, 2023

Would it be possible to backport to .NET 8 if it's a limited change: only adding <auto-generated> comment?

@jonathanpeppers
Copy link
Member

Yes, we can consider taking this one to .NET 8 servicing after it's merged:

dellis1972 added a commit to dellis1972/xamarin-android that referenced this issue Jan 4, 2024
…ings

Fixes dotnet#7480

* Remove the use of `PreserveAttribute` as we no longer support Xamarin
  Classic in main.
* Add #pragma and code comments to fix certain CS* warnings which are
  emitted by the C# compiler.
* Ignore a bunch of other warnings in the unit test.
dellis1972 added a commit to dellis1972/xamarin-android that referenced this issue Feb 16, 2024
…ings

Fixes dotnet#7480

* Remove the use of `PreserveAttribute` as we no longer support Xamarin
  Classic in main.
* Add #pragma and code comments to fix certain CS* warnings which are
  emitted by the C# compiler.
* Ignore a bunch of other warnings in the unit test.
dellis1972 added a commit to dellis1972/xamarin-android that referenced this issue Feb 20, 2024
…ings

Fixes dotnet#7480

* Remove the use of `PreserveAttribute` as we no longer support Xamarin
  Classic in main.
* Add #pragma and code comments to fix certain CS* warnings which are
  emitted by the C# compiler.
* Ignore a bunch of other warnings in the unit test.
dellis1972 added a commit to dellis1972/xamarin-android that referenced this issue Mar 1, 2024
…ings

Fixes dotnet#7480

* Remove the use of `PreserveAttribute` as we no longer support Xamarin
  Classic in main.
* Add #pragma and code comments to fix certain CS* warnings which are
  emitted by the C# compiler.
* Ignore a bunch of other warnings in the unit test.
dellis1972 added a commit to dellis1972/xamarin-android that referenced this issue Mar 4, 2024
…ings

Fixes dotnet#7480

* Remove the use of `PreserveAttribute` as we no longer support Xamarin
  Classic in main.
* Add #pragma and code comments to fix certain CS* warnings which are
  emitted by the C# compiler.
* Ignore a bunch of other warnings in the unit test.
jonpryor pushed a commit that referenced this issue Mar 27, 2024
Fixes: #7480

Context: e604833

When [Layout Bindings][0] are enabled, a set of `partial` classes
will be generated which "mirrors" the `.axml` structure in C#.

Consider:

	dotnet new android -n com.example.codebehind

You enable Code-Behind by:

 1. Setting the `$(AndroidGenerateLayoutBindings)` MSBuild property
    to `true`.
 2. Using `android:id` on elements within `.axml` files.

Consider this patch to the above `dotnet new`:

	diff --git a/Resources/layout/activity_main.xml b/Resources/layout/activity_main.xml
	index f949852..c74521c 100644
	--- a/Resources/layout/activity_main.xml
	+++ b/Resources/layout/activity_main.xml
	@@ -8,6 +8,7 @@
	         android:layout_width="wrap_content"
	         android:layout_height="wrap_content"
	         android:layout_centerInParent="true"
	+        android:id="@+id/text"
	         android:text="@string/app_text"
	     />
	 </RelativeLayout>
	diff --git a/com.example.codebehind.csproj b/com.example.codebehind.csproj
	index 3fdbcb5..8190f84 100644
	--- a/com.example.codebehind.csproj
	+++ b/com.example.codebehind.csproj
	@@ -8,5 +8,6 @@
	     <ApplicationId>com.companyname.com.example.codebehind</ApplicationId>
	     <ApplicationVersion>1</ApplicationVersion>
	     <ApplicationDisplayVersion>1.0</ApplicationDisplayVersion>
	+    <AndroidGenerateLayoutBindings>true</AndroidGenerateLayoutBindings>
	   </PropertyGroup>
	 </Project>

The resulting output contains
`$(IntermediateOutputPath)codebehind\Binding.activity_main.g.cs`,
which declares a type based on the `.axml` file name, and a mirror of
the structure:

	namespace Binding {
	    // typename based on `activity_main.xml` filename
	    sealed partial class activity_main : global::Xamarin.Android.Design.LayoutBinding {
	        [global::Android.Runtime.PreserveAttribute (Conditional=true)]
	        public activity_main (Activity client, …);
	        // …

	        // `text` is via `android:id="@+id/text"`
	        public TextView text => …;
	    }
	}

The problem is the use of `PreserveAttribute` in the generated code;
it's been `[Obsolete]` since e604833 (over two years ago), which
means all projects using Layout Bindings and Layout Code-Behind always
have [CS0618][1] warnings.

Remove the emission of `PreserveAttribute`, so that CS0618 is no
longer generated.

Additionally, within the generated files disable the [CS1591][2] and
[CS8981][3] warnings:

  * CS1591 is around missing XML documentation comments.  These code
    behind files do not have XML documentation comments, so if/when
    `$(DocumentationFile)` is set, these will be emitted for the
    generated code.  We do not currently plan on emitting XML
    documentation comments, so disable this warning.

  * CS8981 is emitted when a type name consists solely of lowercase
    characters.  As the generated Binding types are based on the
    filename -- which is frequently all lowercase -- this warning
    may be emitted.

Finally, add `#nullable enable` to `LayoutBinding.cs` and fix the
nullable reference type warnings.

[0]: https://github.com/xamarin/xamarin-android/blob/2f4e01ec15102dd9cd922cbd833f6482d69512b5/Documentation/guides/LayoutCodeBehind.md
[1]: https://learn.microsoft.com/dotnet/csharp/language-reference/compiler-messages/cs0618
[2]: https://learn.microsoft.com/dotnet/csharp/language-reference/compiler-messages/cs1591
[3]: https://learn.microsoft.com/dotnet/csharp/language-reference/compiler-messages/warning-waves#cs8981---the-type-name-only-contains-lower-cased-ascii-characters
@github-actions github-actions bot locked and limited conversation to collaborators Apr 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Area: App+Library Build Issues when building Library projects or Application projects.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants