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

Handle properties with getter or setter only [Obsolete] attribute #1033

Closed
jpobst opened this issue Aug 30, 2022 · 0 comments · Fixed by #1062
Closed

Handle properties with getter or setter only [Obsolete] attribute #1033

jpobst opened this issue Aug 30, 2022 · 0 comments · Fixed by #1062
Labels
enhancement Proposed change to current functionality generator Issues binding a Java library (generator, class-parse, etc.)
Milestone

Comments

@jpobst
Copy link
Contributor

jpobst commented Aug 30, 2022

When we are converting Java getter/setter pairs to C# properties, we can hit an interesting scenario where a getter may be @Deprecated and the setter is not, or vice versa:

  public boolean hasOptionsMenu () { ... }

  @Deprecated
  public void setHasOptionsMenu (boolean hasMenu) { ... }

C# has traditionally not allowed [Obsolete] to be placed on just a get or a set, it can only be placed on the entire property.

  [Obsolete]
  public bool HasOptionsMenu { get; set; }

This can lead to confusion because using the getter will report an obsolete warning when it is not obsolete. Thus, for properties, we only add [Obsolete] in 2 cases:

  • The get is obsolete and there is no set
  • Both the get and set are obsolete

We have this comment in our code:

// Unlike [Register], [Obsolete] cannot be put on property accessors, so we can apply them only under limited condition...

However, the compiler team has determined that preventing [Obsolete] on property accessors was a bug, and has fixed it in C# 8: dotnet/roslyn#32571.

Thus we can update generator to support scenarios in which only the Java getter or setter is marked as @Deprecated.

@jpobst jpobst added enhancement Proposed change to current functionality generator Issues binding a Java library (generator, class-parse, etc.) labels Aug 30, 2022
@jpobst jpobst added this to the 8.0.0 milestone Sep 1, 2022
jonpryor pushed a commit that referenced this issue Dec 14, 2022
Fixes: #1033

Context: dotnet/roslyn#32571

When we are converting Java get-method and set-method pairs to
C# properties, we can hit an interesting scenario where a getter may
be `@Deprecated` and the setter is not, or vice versa:

	class Example {
	  public boolean hasOptionsMenu () { ... }

	  @deprecated
	  public void setHasOptionsMenu (boolean hasMenu) { ... }
	}

C# has traditionally not allowed `[Obsolete]` to be placed on just a
`get` or a `set` block; it could only be on the entire property:

	partial class Example {
	  [Obsolete]
	  public bool HasOptionsMenu { get; set; }
	}

This can lead to confusion because using the getter will report an
obsolete warning when it is not obsolete.  Thus, for properties, we
only add `[Obsolete]` in 2 cases:

 1. The `get` is obsolete and there is no `set`
 2. Both the `get` and `set` are obsolete

We have this comment in our code:

	// Unlike [Register], [Obsolete] cannot be put on property accessors, so we can apply them only under limited condition...

However, the C# compiler team has determined that preventing
`[Obsolete]` on property accessors was a bug, and has fixed it in C#8
via dotnet/roslyn#32571.

Update `generator` to support scenarios in which only the Java getter
or setter is marked as `@Deprecated`, by placing `[Obsolete]` on the
appropriate `get` or `set` block:

	partial class Example {
	    public bool HasOptionsMenu {
	        get => …

	        [Obsolete]
	        set =>…
	    }
	}
@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
Development

Successfully merging a pull request may close this issue.

1 participant