Move Editor Attribute to S.CM.Primitives and apply to types that had it in netfx by safern · Pull Request #41145 · dotnet/runtime

safern · 2020-08-21T08:41:49Z

Also, I fixed a couple of type full names on DesignerSerializableAttributes that were missing Culture=neutral.

…erSerializableAttributes

Dotnet-GitSync-Bot · 2020-08-21T08:41:54Z

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-21T08:41:55Z

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.

ericstj · 2020-08-21T15:45:47Z

src/libraries/System.ComponentModel.TypeConverter/ref/System.ComponentModel.TypeConverter.cs

        protected virtual bool ShowDialog(System.ComponentModel.Design.DesignerOptionService.DesignerOptionCollection options, object optionObject) { throw null; }
        object System.ComponentModel.Design.IDesignerOptionService.GetOptionValue(string pageName, string valueName) { throw null; }
        void System.ComponentModel.Design.IDesignerOptionService.SetOptionValue(string pageName, string valueName, object value) { }
+        [System.ComponentModel.EditorAttribute("", "System.Drawing.Design.UITypeEditor, System.Drawing, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a")]


Empty string looks odd but that seems consistent with .NETFramework

I guess if we wanted we could have UITypeEditor as the editor type and base type empty?

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

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

...raries/System.Collections.Specialized/src/System/Collections/Specialized/StringDictionary.cs

eerhardt · 2020-08-21T16:23:41Z

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

-        System.Data.IDataReader System.Data.IDbCommand.ExecuteReader() { throw null; }
-        System.Data.IDataReader System.Data.IDbCommand.ExecuteReader(System.Data.CommandBehavior behavior) { throw null; }
-        object System.ICloneable.Clone() { throw null; }
+        System.Data.IDataReader? System.Data.IDbCommand.ExecuteReader() { throw null; }


Was this just a missed auto-gen the ref code from a previous commit?

Something is wrong here.

runtime/src/libraries/System.Data.Common/src/System/Data/IDbCommand.cs

Lines 22 to 23 in 89292fe

IDataReader ExecuteReader();

IDataReader ExecuteReader(CommandBehavior behavior);

runtime/src/libraries/System.Data.OleDb/src/OleDbCommand.cs

Lines 605 to 608 in 89292fe

IDataReader IDbCommand.ExecuteReader()

{

return ExecuteReader(CommandBehavior.Default);

}

runtime/src/libraries/System.Data.OleDb/src/OleDbCommand.cs

Lines 616 to 619 in 89292fe

IDataReader IDbCommand.ExecuteReader(CommandBehavior behavior)

{

return ExecuteReader(behavior);

}

None of these are marked as nullable.

I guess GenAPI has a bug where it is finding a NullableContextAttribute(2) in the same context given that these are non-visible members and we tell Roslyn to not preserve Nullable* metadata for non-visible members. Maybe we should just skip these members in GenAPI or just match the interface annotation instead.

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

eerhardt · 2020-08-21T18:45:53Z

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

-        object System.ICloneable.Clone() { throw null; }
+        System.Data.IDataReader? System.Data.IDbCommand.ExecuteReader() { throw null; }
+        System.Data.IDataReader? System.Data.IDbCommand.ExecuteReader(System.Data.CommandBehavior behavior) { throw null; }
+        object? System.ICloneable.Clone() { throw null; }


Even the Clone() method appears to be wrong:

runtime/src/libraries/System.Private.CoreLib/src/System/ICloneable.cs

Lines 6 to 9 in aa5fdab

public interface ICloneable

{

object Clone();

}

eerhardt

LGTM. Thanks!

safern · 2020-08-21T21:35:02Z

C:\h\w\A41C090F\w\B544097D\e>"C:\h\w\A41C090F\p\dotnet.exe" exec --runtimeconfig Common.Tests.runtimeconfig.json --depsfile Common.Tests.deps.json xunit.console.dll Common.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing  
Failed to load the dll from [C:\h\w\A41C090F\p\shared\Microsoft.NETCore.App\6.0.0\coreclr.dll], HRESULT: 0x8007007F
Failed to bind to CoreCLR at 'C:\h\w\A41C090F\p\shared\Microsoft.NETCore.App\6.0.0\'
Failed to create CoreCLR, HRESULT: 0x80008088

@jkotas I've never seen this before. I downloaded the payload and I do see coreclr.dll in the path it looked for it.

safern · 2020-08-23T00:23:46Z

Failures are: #41215

…it in netfx (dotnet#41145) * Move EditorAttribute to System.ComponentModel.Primitives * Add EditorAttribute to types that had it in netfx and fix some DesignerSerializableAttributes * PR Feedback * PR Feedback

…it in netfx (#41145) (#41289) * Move EditorAttribute to System.ComponentModel.Primitives * Add EditorAttribute to types that had it in netfx and fix some DesignerSerializableAttributes * PR Feedback * PR Feedback

safern added 2 commits August 20, 2020 13:30

Move EditorAttribute to System.ComponentModel.Primitives

4005d34

Add EditorAttribute to types that had it in netfx and fix some Design…

374ff8c

…erSerializableAttributes

safern requested review from eerhardt and ericstj August 21, 2020 08:41

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

danmoseley added area-Meta and removed new-api-needs-documentation labels Aug 21, 2020

ericstj reviewed Aug 21, 2020

View reviewed changes

safern commented Aug 21, 2020

View reviewed changes

src/libraries/System.Drawing.Common/ref/System.Drawing.Common.cs Show resolved Hide resolved

eerhardt reviewed Aug 21, 2020

View reviewed changes

...raries/System.Collections.Specialized/src/System/Collections/Specialized/StringDictionary.cs Show resolved Hide resolved

eerhardt reviewed Aug 21, 2020

View reviewed changes

src/libraries/System.Drawing.Common/ref/System.Drawing.Common.cs Show resolved Hide resolved

PR Feedback

4d9de26

ericstj approved these changes Aug 21, 2020

View reviewed changes

eerhardt reviewed Aug 21, 2020

View reviewed changes

PR Feedback

3e54901

eerhardt approved these changes Aug 21, 2020

View reviewed changes

safern mentioned this pull request Aug 21, 2020

[Gen-API] generating wrong nullable annotations for some explicit implementations dotnet/arcade#6016

Open

safern merged commit 3130ac9 into dotnet:master Aug 23, 2020

safern deleted the EditorAttribute branch August 23, 2020 00:24

safern mentioned this pull request Aug 24, 2020

[release/5.0] Move Editor Attribute to S.CM.Primitives and apply to types that had it in netfx #41289

Merged

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

	IDataReader ExecuteReader();
	IDataReader ExecuteReader(CommandBehavior behavior);

	IDataReader IDbCommand.ExecuteReader()
	{
	return ExecuteReader(CommandBehavior.Default);
	}

	IDataReader IDbCommand.ExecuteReader(CommandBehavior behavior)
	{
	return ExecuteReader(behavior);
	}

Conversation

safern commented Aug 21, 2020

Uh oh!

Dotnet-GitSync-Bot commented Aug 21, 2020

Uh oh!

Dotnet-GitSync-Bot commented Aug 21, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eerhardt left a comment

Choose a reason for hiding this comment

Uh oh!

safern commented Aug 21, 2020

Uh oh!

safern commented Aug 23, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants