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

[Xamarin.Android.Build.Tasks] Add support for $(AndroidEnableObsoleteOverrideInheritance). #8393

Merged
merged 1 commit into from
Oct 10, 2023

Conversation

jpobst
Copy link
Contributor

@jpobst jpobst commented Oct 3, 2023

Context: dotnet/java-interop#1143
Context: dotnet/java-interop#1130

In dotnet/java-interop#1143, we added the ability to globally opt-out of the new generator feature that automatically applies [Obsolete] attributes to methods that override [Obsolete] methods. (Preventing a C# compiler warning.)

This adds support for a new $(AndroidEnableObsoleteOverrideInheritance) boolean MSBuild property for users to opt-out. The default is true.

@jpobst jpobst marked this pull request as ready for review October 5, 2023 20:48
@@ -469,6 +469,15 @@ Support for this property was added in Xamarin.Android 5.1.

This property is `False` by default.

## AndroidEnableObsoleteOverrideInheritance
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. We do not expect ~anyone to actually use this, however there was an internal customer with a very custom setup that hit this and we did need to provide an opt-out. Thus we do not intend to widely document or suggest its usage.

@jonpryor jonpryor merged commit 1a309b6 into main Oct 10, 2023
47 checks passed
@jonpryor jonpryor deleted the obsolete-inheritance branch October 10, 2023 00:09
jonathanpeppers pushed a commit that referenced this pull request Oct 10, 2023
…OverrideInheritance). (#8393)

Context: dotnet/java-interop@8e63cc8
Context: dotnet/java-interop@d0231c5

dotnet/java-interop@d0231c5c updated generator output so that
`[Obsolete]` would automagically "flow" to derived types and method
overrides, so that [warning CS0672][0] would not be emitted.

Before dotnet/java-interop@d0231c5c, we would emit:

	public class Context {
	  [Obsolete]
	  public virtual void SetWallpaper () { … }
	}

	public class ContextWrapper : Context {
	  [ObsoletedOSPlatform ("android23.0")]
	  public override void SetWallpaper () { … }
	}

`ContextWrapper.SetWallpaper()` would elicit a CS0672 warning:

	warning CS0672: Member 'ContextWrapper.SetWallpaper()' overrides obsolete member 'Context.SetWallpaper()'.
	Add the Obsolete attribute to 'ContextWrapper.SetWallpaper()'

With dotnet/java-interop@d0231c5c, we now emit the following, which
no longer emits CS0672:

	public class Context {
	  [Obsolete]
	  public virtual void SetWallpaper () { … }
	}

	public class ContextWrapper : Context {
	  [Obsolete]
	  public override void SetWallpaper () { … }
	}

While we feel that this is a nice improvement, this broke some
customers who didn't want their bindings to automatically gain
`[Obsolete]` "just because" they were overriding `[Obsolete]`
types or members.

In dotnet/java-interop@8e63cc8d, we added the ability to globally
*opt-out* of this new behavior by using
`generator --lang-features=do-not-fix-obsolete-overrides`.

Add a new `$(AndroidEnableObsoleteOverrideInheritance)` MSBuild
property.  When False, this adds
`generator --lang-features=do-not-fix-obsolete-overrides`,
preventing the automatic CS0672 fix behavior.

By default `$(AndroidEnableObsoleteOverrideInheritance)` is True.

[0]: https://learn.microsoft.com/en-us/dotnet/csharp/misc/cs0672
grendello added a commit to grendello/xamarin-android that referenced this pull request Oct 10, 2023
* main:
  [Xamarin.Android.Build.Tasks] Add support for $(AndroidEnableObsoleteOverrideInheritance). (dotnet#8393)
  [Xamarin.Android.Build.Tasks] Make manifestmerger.jar the default. (dotnet#8392)
jonathanpeppers pushed a commit that referenced this pull request Oct 17, 2023
…OverrideInheritance). (#8393)

Context: dotnet/java-interop@8e63cc8
Context: dotnet/java-interop@d0231c5

dotnet/java-interop@d0231c5c updated generator output so that
`[Obsolete]` would automagically "flow" to derived types and method
overrides, so that [warning CS0672][0] would not be emitted.

Before dotnet/java-interop@d0231c5c, we would emit:

	public class Context {
	  [Obsolete]
	  public virtual void SetWallpaper () { … }
	}

	public class ContextWrapper : Context {
	  [ObsoletedOSPlatform ("android23.0")]
	  public override void SetWallpaper () { … }
	}

`ContextWrapper.SetWallpaper()` would elicit a CS0672 warning:

	warning CS0672: Member 'ContextWrapper.SetWallpaper()' overrides obsolete member 'Context.SetWallpaper()'.
	Add the Obsolete attribute to 'ContextWrapper.SetWallpaper()'

With dotnet/java-interop@d0231c5c, we now emit the following, which
no longer emits CS0672:

	public class Context {
	  [Obsolete]
	  public virtual void SetWallpaper () { … }
	}

	public class ContextWrapper : Context {
	  [Obsolete]
	  public override void SetWallpaper () { … }
	}

While we feel that this is a nice improvement, this broke some
customers who didn't want their bindings to automatically gain
`[Obsolete]` "just because" they were overriding `[Obsolete]`
types or members.

In dotnet/java-interop@8e63cc8d, we added the ability to globally
*opt-out* of this new behavior by using
`generator --lang-features=do-not-fix-obsolete-overrides`.

Add a new `$(AndroidEnableObsoleteOverrideInheritance)` MSBuild
property.  When False, this adds
`generator --lang-features=do-not-fix-obsolete-overrides`,
preventing the automatic CS0672 fix behavior.

By default `$(AndroidEnableObsoleteOverrideInheritance)` is True.

[0]: https://learn.microsoft.com/en-us/dotnet/csharp/misc/cs0672
@github-actions github-actions bot locked and limited conversation to collaborators Jan 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants