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

[generator] Don't re-add members as 'virtual' or 'new virtual' when conforming to protocol #3217

Open
VincentDondain opened this issue Jan 12, 2018 · 1 comment
Labels
bug If an issue is a bug or a pull request a bug fix generator Issues affecting the generator good first issue This is a good first issue for someone to start working with our code iOS Issues affecting Xamarin.iOS macOS Issues affecting Xamarin.Mac
Milestone

Comments

@VincentDondain
Copy link
Contributor

Steps to Reproduce

CNKeyDescriptor conforms to NSObjectProtocol and when I added CNKeyDescriptor conformance to NSString it re-decalred a bunch of members that were already there (from NSObject).

See generate code.

Expected Behavior

The generator should be smart enough to not re-generate members that are already defined in the base type.

Actual Behavior

Code is duplicated.

Environment

=== Visual Studio Community 2017 for Mac (Preview) ===

Version 7.4 Preview (7.4 build 720)
Installation UUID: 00d602ef-271b-479b-9054-0433b5797215
Runtime:
	Mono 5.8.0.99 (2017-10/e65bf00e22b) (64-bit)
	GTK+ 2.24.23 (Raleigh theme)

	Package version: 508000099

=== NuGet ===

Version: 4.3.1.4445

=== .NET Core ===

Runtime: /usr/local/share/dotnet/dotnet
Runtime Versions:
	2.0.0-preview2-25407-01
	1.1.2
	1.0.5
SDK: /usr/local/share/dotnet/sdk/2.0.0-preview2-006497/Sdks
SDK Versions:
	2.0.0-preview2-006497
	1.0.4
MSBuild SDKs: /Library/Frameworks/Mono.framework/Versions/5.8.0/lib/mono/msbuild/15.0/bin/Sdks

=== Xamarin.Profiler ===

Version: 1.5.6
Location: /Applications/Xamarin Profiler.app/Contents/MacOS/Xamarin Profiler

=== Apple Developer Tools ===

Xcode 9.2 (13772)
Build 9C40b

=== Xamarin.Mac ===

Version: 4.3.0.17 (Visual Studio Community)

=== Xamarin.iOS ===

Version: 11.9.0.17 (Visual Studio Community)
Hash: 57ef7c4d
Branch: bug-59272
Build date: 2018-01-12 13:44:41-0500

=== Xamarin.Android ===

Version: 8.0.2.1 (Visual Studio Community)
Android SDK: /Users/vidondai/Library/Developer/Xamarin/android-sdk-macosx
	Supported Android versions:
		6.0 (API level 23)
		7.1 (API level 25)
		8.0 (API level 26)

SDK Tools Version: 25.2.5
SDK Platform Tools Version: 25.0.4
SDK Build Tools Version: 23.0.2

Java SDK: /usr
java version "1.8.0_101"
Java(TM) SE Runtime Environment (build 1.8.0_101-b13)
Java HotSpot(TM) 64-Bit Server VM (build 25.101-b13, mixed mode)

Android Designer EPL code available here:
https://github.com/xamarin/AndroidDesigner.EPL

=== Xamarin Inspector ===

Version: 1.3.2
Hash: 461f09a
Branch: 1.3-release
Build date: Tue, 03 Oct 2017 18:26:57 GMT
Client compatibility: 1

=== Build Information ===

Release ID: 704000720
Git revision: d5d46cb994643f64cc683737ecef76f581cf2b89
Build date: 2017-11-28 09:24:00-05
Xamarin addins: e7e3c0e88c36edf0498dba75a52e3c0654fe69ed
Build lane: monodevelop-lion-master

=== Operating System ===

Mac OS X 10.12.6
Darwin 16.7.0 Darwin Kernel Version 16.7.0
    Wed Oct  4 00:17:00 PDT 2017
    root:xnu-3789.71.6~1/RELEASE_X86_64 x86_64

=== Enabled user installed addins ===

NuGet Package Explorer 0.2
Internet of Things (IoT) development (Preview) 7.1

@VincentDondain VincentDondain added the bug If an issue is a bug or a pull request a bug fix label Jan 12, 2018
@spouliot
Copy link
Contributor

To be more accurate: source code is duplicated, but binary-wise those duplicated methods ends up being new virtual which could cause hard to diagnose problems when subclassing NSString

@spouliot spouliot added macOS Issues affecting Xamarin.Mac iOS Issues affecting Xamarin.iOS labels Jan 12, 2018
@spouliot spouliot added this to the Future milestone Jan 12, 2018
VincentDondain added a commit to VincentDondain/xamarin-macios that referenced this issue Jan 12, 2018
VincentDondain added a commit that referenced this issue Jan 18, 2018
#3187)

- Fixes bug #59272: [xtro] Report !missing-protocol-conformance! when protocols are defined in categories
(https://bugzilla.xamarin.com/show_bug.cgi?id=59272)
- Implemented missing protocol conformances based on tool's new data.
- Remove Internal check from VisitObjCCategoryDecl and VisitObjCInterfaceDecl
- Ignore previewItemTitle failure (normal since it's optional)
- Only skip UIStateRestoring for subclasses of UIViewController
- Ignore UIStateRestoring test on watchOS (UIViewController not available)
- Remove protocol conformances that generated wrong availability attributes (#3213)
- Avoid new virtual or virtual when adding protocol conformance (#3217)
@rolfbjarne rolfbjarne added good first issue This is a good first issue for someone to start working with our code generator Issues affecting the generator labels Sep 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug If an issue is a bug or a pull request a bug fix generator Issues affecting the generator good first issue This is a good first issue for someone to start working with our code iOS Issues affecting Xamarin.iOS macOS Issues affecting Xamarin.Mac
Projects
None yet
Development

No branches or pull requests

3 participants