[wasm] Mark System.Console APIs as unsupported on Browser by mdh1418 · Pull Request #41184 · dotnet/runtime

mdh1418 · 2020-08-21T21:02:44Z

Contributes to #41087

browser M:System.Console.set_BufferWidth(System.Int32),System,Console,set_BufferWidth(Int32),1 
windows "M:System.Console.MoveBufferArea(System.Int32,System.Int32,System.Int32,System.Int32,System.Int32,System.Int32,System.Char,System.ConsoleColor,System.ConsoleColor)",System,Console,"MoveBufferArea(Int32, Int32, Int32, Int32, Int32, Int32, Char, ConsoleColor, ConsoleColor)",1
windows M:System.Console.set_WindowTop(System.Int32),System,Console,set_WindowTop(Int32),1
browser M:System.Console.add_CancelKeyPress(System.ConsoleCancelEventHandler),System,Console,add_CancelKeyPress(ConsoleCancelEventHandler),1
browser M:System.Console.Beep,System,Console,Beep(),1
browser M:System.Console.get_ForegroundColor,System,Console,get_ForegroundColor(),1
browser M:System.Console.set_CursorVisible(System.Boolean),System,Console,set_CursorVisible(Boolean),1
browser M:System.Console.get_In,System,Console,get_In(),2
browser M:System.Console.set_CursorTop(System.Int32),System,Console,set_CursorTop(Int32),2
browser M:System.Console.get_CursorSize,System,Console,get_CursorSize(),1
browser M:System.Console.set_WindowWidth(System.Int32),System,Console,set_WindowWidth(Int32),1
browser M:System.Console.get_CursorLeft,System,Console,get_CursorLeft(),1
browser M:System.Console.ResetColor,System,Console,ResetColor(),1
browser M:System.Console.get_CursorVisible,System,Console,get_CursorVisible(),1
browser "M:System.Console.SetCursorPosition(System.Int32,System.Int32)",System,Console,"SetCursorPosition(Int32, Int32)",1
browser M:System.Console.OpenStandardInput,System,Console,OpenStandardInput(),1
browser M:System.Console.set_WindowHeight(System.Int32),System,Console,set_WindowHeight(Int32),1
windows "M:System.Console.SetBufferSize(System.Int32,System.Int32)",System,Console,"SetBufferSize(Int32, Int32)",1
browser M:System.Console.set_CursorLeft(System.Int32),System,Console,set_CursorLeft(Int32),2
browser M:System.Console.remove_CancelKeyPress(System.ConsoleCancelEventHandler),System,Console,remove_CancelKeyPress(ConsoleCancelEventHandler),1
browser M:System.Console.get_Title,System,Console,get_Title(),1
browser M:System.Console.set_InputEncoding(System.Text.Encoding),System,Console,set_InputEncoding(Encoding),1
windows "M:System.Console.Beep(System.Int32,System.Int32)",System,Console,"Beep(Int32, Int32)",1
browser M:System.Console.get_CursorTop,System,Console,get_CursorTop(),1
windows "M:System.Console.MoveBufferArea(System.Int32,System.Int32,System.Int32,System.Int32,System.Int32,System.Int32)",System,Console,"MoveBufferArea(Int32, Int32, Int32, Int32, Int32, Int32)",1
browser M:System.Console.get_WindowWidth,System,Console,get_WindowWidth(),1
browser M:System.Console.get_LargestWindowHeight,System,Console,get_LargestWindowHeight(),1
browser M:System.Console.set_TreatControlCAsInput(System.Boolean),System,Console,set_TreatControlCAsInput(Boolean),1
windows "M:System.Console.SetWindowSize(System.Int32,System.Int32)",System,Console,"SetWindowSize(Int32, Int32)",1
windows M:System.Console.get_NumberLock,System,Console,get_NumberLock(),1
browser M:System.Console.ReadKey,System,Console,ReadKey(),1
browser M:System.Console.get_BufferHeight,System,Console,get_BufferHeight(),1
browser M:System.Console.get_BufferWidth,System,Console,get_BufferWidth(),1
browser M:System.Console.ReadKey(System.Boolean),System,Console,ReadKey(Boolean),1
browser M:System.Console.get_InputEncoding,System,Console,get_InputEncoding(),1
browser M:System.Console.ReadLine,System,Console,ReadLine(),3
browser M:System.Console.get_WindowHeight,System,Console,get_WindowHeight(),1
browser M:System.Console.set_BackgroundColor(System.ConsoleColor),System,Console,set_BackgroundColor(ConsoleColor),1
browser M:System.Console.set_ForegroundColor(System.ConsoleColor),System,Console,set_ForegroundColor(ConsoleColor),1
browser M:System.Console.set_CursorSize(System.Int32),System,Console,set_CursorSize(Int32),1
windows M:System.Console.get_CapsLock,System,Console,get_CapsLock(),1
windows "M:System.Console.SetWindowPosition(System.Int32,System.Int32)",System,Console,"SetWindowPosition(Int32, Int32)",1
browser M:System.Console.OpenStandardInput(System.Int32),System,Console,OpenStandardInput(Int32),2
browser M:System.Console.set_BufferHeight(System.Int32),System,Console,set_BufferHeight(Int32),1
browser M:System.Console.get_BackgroundColor,System,Console,get_BackgroundColor(),1
browser M:System.Console.Read,System,Console,Read(),3
browser M:System.Console.GetCursorPosition,System,Console,GetCursorPosition(),1
browser M:System.Console.Clear,System,Console,Clear(),1
browser M:System.Console.get_TreatControlCAsInput,System,Console,get_TreatControlCAsInput(),1
browser M:System.Console.set_Title(System.String),System,Console,set_Title(String),1
windows M:System.Console.set_WindowLeft(System.Int32),System,Console,set_WindowLeft(Int32),1
browser M:System.Console.get_LargestWindowWidth,System,Console,get_LargestWindowWidth(),1

Dotnet-GitSync-Bot · 2020-08-21T21:02:47Z

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.

ghost · 2020-08-21T21:03:03Z

Tagging subscribers to this area: @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

src/libraries/System.Console/ref/System.Console.cs

mdh1418 · 2020-09-03T21:50:43Z

src/libraries/System.Console/ref/System.Console.cs

I'm not sure is this would be considered an "inconsistent list." Should [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] instead be applied right before set?

I just posted a similar question on the src file before seeing this comment. #41184 (comment)

jeffhandley · 2020-09-03T21:56:16Z

src/libraries/System.Console/src/System/Console.cs

This is an interesting scenario! @buyaa-n can you weigh in on how this will be interpreted with the property being marked as [Unsupported("browser")] but the get method having [SupportedOSPlatform("windows")]? Is this OK or should we instead mark the set method as [UnsupportedOSPlatform("browser")]?

Is the UnsupportedOSPlatform even necessary? I imagine that the behavior of the analyzer would be "if there is a SupportedOSPlatformAttribute" it will flag the API as unsupported on any other platforms?

Maybe we should just mark the whole property as SupportedOSPlatform("windows")? It seems weird to be able to set a property that you can't get on the same platform? Or does this property affects other APIs that are supported?

What was intended here was to mark the property getter as supported only on Windows, but the property setter as supported everywhere (except browser now). The getter and setter can have their own attributes. But I don't know what the analyzer will do when the getter/setter differ from the property itself like this.

It might be clearer to just mark the set method as [UnsupportedOSPlatform("browser")] even if this would actually work as-is.

I see. Maybe something to consider on the Analyzer? That if the property has an attribute, but the getter/setter has another, the getter/setter wins?

This is an interesting scenario! @buyaa-n can you weigh in on how this will be interpreted with the property being marked as [Unsupported("browser")] but the get method having [SupportedOSPlatform("windows")]? Is this OK or should we instead mark the set method as [UnsupportedOSPlatform("browser")]?

Yes it would be considered as an inconsistent list, as you mentioned the [UnsupportedOSPlatform("browser")] attribute better to be in the setter.

I see. Maybe something to consider on the Analyzer? That if the property has an attribute, but the getter/setter has another, the getter/setter wins?

It works that way in case they are for the same platform, but not for different platform

Cool. @mdh1418 per these comments, please update all properties where there was already [SupportedOSPlatform("windows")] on one of the accessors but not the other, and move the [UnsupportedOSPlatform("browser")] attribute down to the other accessor instead of on the property itself.

jeffhandley

move UnsupportedOSPlatform attributes down to the accessor

Sorry, I probably wasn't clear enough in my previous comment. We should only move attributes down to the accessors when there are existing attributes on the other accessor.

For properties only with get, the attribute can be on the property itself. And for properties with both get and set that did not previously have [SupportedOSPlatform("windows")] you should also apply the new attribute to the entire property (assuming both accessors are unsupported on browser).

We only need to move the attribute to the accessors when support between the 2 accessors differs.

…ssor

mdh1418 · 2020-09-04T18:59:40Z

For properties only with get, the attribute can be on the property itself. And for properties with both get and set that did not previously have [SupportedOSPlatform("windows")] you should also apply the new attribute to the entire property (assuming both accessors are unsupported on browser).

Ah, right, so the attribute will be applied to the highest level. I removed a commit that had previously moved the attributes down to the accessor level.

jeffhandley

Thanks!! This is exciting to see.

ghost · 2020-09-04T19:53:40Z

Hello @safern!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

* [wasm] Mark System.Console APIs as unsupported on Browser * System.Console remove unsupported attribute from Console.Clear() * System.Console mark SetIn as unsupported on Browser * System.Console move UnsupportedOSPlatform attributes down to the accessor Co-authored-by: Mitchell Hwang <[email protected]>

* [wasm] Mark System.ComponentModel APIs as unsupported on Browser (#41094) * [wasm] System.ComponentModel enable platform attributes * [wasm] Mark ExtendedProtectionPolicyTypeConverter.ConvertTo as unsupported * [wasm] Mark System.ComponentModel.TypeDescriptor.CreateInstance as unsupported * [wasm] Mark System.ComponentModel.TypeDescriptionProvider.CreateInstance as unsupported * [wasm] Mark System.ComponentModel.LicenseManager.CreateWithContext as unsupported * [wasm] Mark System.ComponentModel.MaskedTextProvider.Clone as unsupported Co-authored-by: Mitchell Hwang <[email protected]> * [wasm] Mark System.Console APIs as unsupported on Browser (#41184) * [wasm] Mark System.Console APIs as unsupported on Browser * System.Console remove unsupported attribute from Console.Clear() * System.Console mark SetIn as unsupported on Browser * System.Console move UnsupportedOSPlatform attributes down to the accessor Co-authored-by: Mitchell Hwang <[email protected]> * Mark System.Diagnostics.FileVersionInfo as unsupported on Browser (#41271) Co-authored-by: Mitchell Hwang <[email protected]> * Mark System.Diagnostics.Process unsupported at assembly level (#41694) Co-authored-by: Mitchell Hwang <[email protected]> * [wasm] Mark System.IO.Compression APIs as unsupported on Browser (#41683) * [wasm] System.IO.Compression.Brotli enable platform attributes * Mark System.IO.Compression.Brotli unsupported at assembly level Co-authored-by: Mitchell Hwang <[email protected]> * [wasm] Mark System.IO.FileSystem.Watcher APIs as unsupported on Browser (#41682) * [wasm] System.IO.FileSystem.Watcher enable platform attributes * Mark System.IO.FileSystem.Watcher unsupported at assembly level Co-authored-by: Mitchell Hwang <[email protected]> * [wasm] Mark System.IO.IsolatedStorage APIs as unsupported on Browser (#41700) * [wasm] System.IO.IsolatedStorage enable platform attributes * Mark System.IO.IsolatedStorage Unsupported at assembly level Co-authored-by: Mitchell Hwang <[email protected]> * Mark some System.Net.* APIs as unsupported on Browser WASM (#40924) * [wasm] Mark System.Threading.ThreadPool unsupported on Browser (#41891) * System.Threading.ThreadPool enable platform attributes * Mark System.Threading.ThreadPool APIs unsupported on browser * System.Threading.ThreadPool Add Unsupported attribute to other ThreadPool files * Remove Unsupported attributes from BindHandle * Add windows Supported Attribute to BindHandle Co-authored-by: Mitchell Hwang <[email protected]> * Mark System.Net.WebSockets.ClientWebSocketOptions APIs as unsupported on Browser (#41963) * Mark System.Net.WebSockets.ClientWebSocketOptions APIs as unsupported on Browser * Add the attributes to non-browser version of ClientWebSocketOptions class to avoid build error * Add using * Include platform attributes * [wasm] Mark System.Net.NameResolution Unsupported at assembly level (#41985) Co-authored-by: Mitchell Hwang <[email protected]> Co-authored-by: Mitchell Hwang <[email protected]> Co-authored-by: Maxim Lipnin <[email protected]>

* [wasm] Mark System.Console APIs as unsupported on Browser * System.Console remove unsupported attribute from Console.Clear() * System.Console mark SetIn as unsupported on Browser * System.Console move UnsupportedOSPlatform attributes down to the accessor Co-authored-by: Mitchell Hwang <[email protected]>

mdh1418 requested review from MaximLipnin, lewing, safern and steveisok August 21, 2020 21:02

mdh1418 added arch-wasm WebAssembly architecture area-Infrastructure-libraries labels Aug 21, 2020

MaximLipnin mentioned this pull request Aug 21, 2020

Mark APIs as unsupported on Browser WASM #41087

Closed

71 tasks

campersau reviewed Aug 23, 2020

View reviewed changes

src/libraries/System.Console/ref/System.Console.cs Outdated Show resolved Hide resolved

marek-safar requested review from EgorBo and akoeplinger August 25, 2020 20:49

[wasm] Mark System.Console APIs as unsupported on Browser

755a99d

mdh1418 force-pushed the mdhwang/system_console_unsupporetd branch from b958789 to 091167b Compare September 3, 2020 00:48

marek-safar reviewed Sep 3, 2020

View reviewed changes

mdh1418 requested review from buyaa-n and jeffhandley September 3, 2020 21:48

mdh1418 commented Sep 3, 2020

View reviewed changes

jeffhandley reviewed Sep 3, 2020

View reviewed changes

jeffhandley reviewed Sep 4, 2020

View reviewed changes

Mitchell Hwang added 3 commits September 4, 2020 14:48

System.Console remove unsupported attribute from Console.Clear()

cae1d87

System.Console mark SetIn as unsupported on Browser

10ebf54

System.Console move UnsupportedOSPlatform attributes down to the acce…

30783d8

…ssor

mdh1418 force-pushed the mdhwang/system_console_unsupporetd branch from 31abd9c to 30783d8 Compare September 4, 2020 18:54

jeffhandley approved these changes Sep 4, 2020

View reviewed changes

safern approved these changes Sep 4, 2020

View reviewed changes

safern added the auto-merge label Sep 4, 2020

ghost merged commit a9f6fd6 into dotnet:master Sep 4, 2020

danmoseley added area-System.Console and removed area-Infrastructure-libraries labels Sep 5, 2020

mdh1418 deleted the mdhwang/system_console_unsupporetd branch September 8, 2020 16:23

mdh1418 mentioned this pull request Sep 10, 2020

[release/5.0-rc2] Backport Browser Unsupported Attribute Changes Properly #42071

Merged

campersau mentioned this pull request Oct 3, 2020

[wasm] Implement System.Console.Clear #43002

Merged

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

This pull request was closed.

Conversation

mdh1418 commented Aug 21, 2020

Uh oh!

Dotnet-GitSync-Bot commented Aug 21, 2020

Uh oh!

ghost commented Aug 21, 2020

Uh oh!

Uh oh!

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!

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!

jeffhandley left a comment

Choose a reason for hiding this comment

Uh oh!

mdh1418 commented Sep 4, 2020

Uh oh!

jeffhandley left a comment

Choose a reason for hiding this comment

Uh oh!

ghost commented Sep 4, 2020

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.