Add Designer attributes to types that had it in full framework (part 2) by safern · Pull Request #40966 · dotnet/runtime

safern · 2020-08-18T04:39:39Z

Contributes towards: #31428

I will do EditorAttribute in a separate PR since that one is a bigger change.

FYI: @DustinCampbell @RussKie @merriemcgaw

@danmosemsft I think we will need to port this to RC1.

…Dictionary

Dotnet-GitSync-Bot · 2020-08-18T04:39:43Z

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

Dotnet-GitSync-Bot · 2020-08-18T04:39:45Z

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

RussKie · 2020-08-18T04:42:23Z

Which branch is this work targeting? We're branching to .NET 6.0 right now.

RussKie · 2020-08-18T04:45:45Z

I will do EditorAttribute in a separate PR since that one is a bigger change.

Are we getting FontEditor back?

safern · 2020-08-18T05:27:02Z

Which branch is this work targeting? We're branching to .NET 6.0 right now.

This is targeting master now, but I plan porting to RC1 branch.

Are we getting FontEditor back?

That one was not in the list I had from: #31428

Is that blocking you guys?

RussKie · 2020-08-18T05:35:52Z

Are we getting FontEditor back?

That one was not in the list I had from: #31428

Is that blocking you guys?

Sorry, my bad. We're missing FontNameEditor as discussed in dotnet/winforms#2104. We had few internal discussions as to how we could fix it on our end, but all possible solutions weren't simple, were quite hacky and nasty.

It'd be great to have it reinstated, if you're doing this for other attributes.

src/libraries/shims/ApiCompatBaseline.netcoreapp.netstandard.txt

...del.Primitives/src/System/ComponentModel/Design/Serialization/DesignerSerializerAttribute.cs

eerhardt · 2020-08-18T16:03:37Z

    [InlineData(null, null)]

Here are tests explicitly passing null to the constructor.

Refers to: src/libraries/System.ComponentModel.Primitives/tests/System/ComponentModel/Design/Serialization/DesignerSerializerAttributeTests.cs:45 in 8329076. [](commit_id = 8329076, deletion_comment = False)

src/libraries/System.Data.Common/ref/System.Data.Common.cs

src/libraries/shims/ApiCompatBaseline.netcoreapp.netstandard.txt

safern · 2020-08-18T17:30:08Z

@ericstj -- it seems like XElement and XAttribute have TypeDescriptionProviderAttribute in the implementation but not in the ref assembly. However, I don't know if we want to add System.ComponentModel.TypeConverter to the XDocument closure.

We have 3 options:

Add S.CM.TypeConverter to the closure.
Move the attribute down to S.CM.Primitives.
Baseline the errors.

eerhardt · 2020-08-18T17:47:54Z

TypeDescriptionProviderAttribute is in System.ObjectModel:

runtime/src/libraries/System.ObjectModel/ref/System.ObjectModel.cs

Lines 208 to 215 in aa5fdab

    
           [System.AttributeUsageAttribute(System.AttributeTargets.Class, Inherited=true)] 
        
           public sealed partial class TypeDescriptionProviderAttribute : System.Attribute 
        
           { 
        
               public TypeDescriptionProviderAttribute([System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] string typeName) { } 
        
               public TypeDescriptionProviderAttribute([System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] System.Type type) { } 
        
               [System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] 
        
               public string TypeName { get { throw null; } } 
        
           }

ericstj · 2020-08-18T17:57:42Z

I believe I specifically considered this in c160716. I think at the time we were opting to be conservative exposing attributes in reference assemblies unless we had a specific scenario for including them. Did we have a specific scenario for TypeDescriptionProviderAttribute?

safern · 2020-08-18T18:00:49Z

TypeDescriptionProviderAttribute is in System.ObjectModel.

Thanks, I should've look deeper.

safern · 2020-08-18T18:06:06Z

Did we have a specific scenario for TypeDescriptionProviderAttribute?

Not really, I was just following what we're doing for all the attributes in:
#31428 (comment)

ericstj · 2020-08-18T18:24:03Z

I see, if its easy and cheap to expose it, go ahead. If there are any problems let's hold off.

eerhardt

src/libraries/System.Collections.Specialized/ref/System.Collections.Specialized.cs

…2) (dotnet#40966) * Move DesignerSerializerAttribute to S.CM.Primitives and add to StringDictionary * Add TolboxItemAttribute to types that had it in Desktop * Remove TypeDescriptionProviderAttribute from DefaultGenApiDocIds * PR Feedback, fix build

…2) (#40966) (#41066) * Move DesignerSerializerAttribute to S.CM.Primitives and add to StringDictionary * Add TolboxItemAttribute to types that had it in Desktop * Remove TypeDescriptionProviderAttribute from DefaultGenApiDocIds * PR Feedback, fix build

safern added 3 commits August 17, 2020 19:40

Move DesignerSerializerAttribute to S.CM.Primitives and add to String…

de9405f

…Dictionary

Add TolboxItemAttribute to types that had it in Desktop

6301132

Remove TypeDescriptionProviderAttribute from DefaultGenApiDocIds

8329076

safern added this to the 5.0.0 milestone Aug 18, 2020

safern requested review from eerhardt and ericstj August 18, 2020 04:39

Dotnet-GitSync-Bot added the new-api-needs-documentation label Aug 18, 2020

eerhardt reviewed Aug 18, 2020

View reviewed changes

src/libraries/shims/ApiCompatBaseline.netcoreapp.netstandard.txt Outdated Show resolved Hide resolved

eerhardt reviewed Aug 18, 2020

View reviewed changes

...del.Primitives/src/System/ComponentModel/Design/Serialization/DesignerSerializerAttribute.cs Outdated Show resolved Hide resolved

ericstj reviewed Aug 18, 2020

View reviewed changes

src/libraries/System.Data.Common/ref/System.Data.Common.cs Outdated Show resolved Hide resolved

src/libraries/shims/ApiCompatBaseline.netcoreapp.netstandard.txt Outdated Show resolved Hide resolved

PR Feedback, fix build

27a812c

ericstj approved these changes Aug 18, 2020

View reviewed changes

eerhardt approved these changes Aug 18, 2020

View reviewed changes

RussKie reviewed Aug 18, 2020

View reviewed changes

src/libraries/System.Collections.Specialized/ref/System.Collections.Specialized.cs Show resolved Hide resolved

safern merged commit 26a71f9 into dotnet:master Aug 19, 2020

safern deleted the DesignerAttributesFullFramework branch August 19, 2020 19:08

safern mentioned this pull request Aug 19, 2020

[release/5.0] Port Add Designer attributes to types that had it in full framework #41066

Merged

safern mentioned this pull request Aug 21, 2020

Move Editor Attribute to S.CM.Primitives and apply to types that had it in netfx #41145

Merged

ghost locked as resolved and limited conversation to collaborators Dec 7, 2020

Conversation

safern commented Aug 18, 2020

Uh oh!

Dotnet-GitSync-Bot commented Aug 18, 2020

Uh oh!

Dotnet-GitSync-Bot commented Aug 18, 2020

Uh oh!

RussKie commented Aug 18, 2020

Uh oh!

RussKie commented Aug 18, 2020

Uh oh!

safern commented Aug 18, 2020

Uh oh!

RussKie commented Aug 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eerhardt commented Aug 18, 2020

Uh oh!

Uh oh!

Uh oh!

safern commented Aug 18, 2020

Uh oh!

eerhardt commented Aug 18, 2020

Uh oh!

ericstj commented Aug 18, 2020

Uh oh!

safern commented Aug 18, 2020

Uh oh!

safern commented Aug 18, 2020

Uh oh!

ericstj commented Aug 18, 2020

Uh oh!

eerhardt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

RussKie commented Aug 18, 2020 •

edited

Loading