{"action":"created","issue":{"url":"https://api.github.com/repos/dart-lang/sdk/issues/38107","repository_url":"https://api.github.com/repos/dart-lang/sdk","labels_url":"https://api.github.com/repos/dart-lang/sdk/issues/38107/labels{/name}","comments_url":"https://api.github.com/repos/dart-lang/sdk/issues/38107/comments","events_url":"https://api.github.com/repos/dart-lang/sdk/issues/38107/events","html_url":"https://github.com/dart-lang/sdk/issues/38107","id":487100045,"node_id":"MDU6SXNzdWU0ODcxMDAwNDU=","number":38107,"title":"Analyzer should block static extensions on @sealed classes","user":{"login":"srawlins","id":103167,"node_id":"MDQ6VXNlcjEwMzE2Nw==","avatar_url":"https://avatars3.githubusercontent.com/u/103167?v=4","gravatar_id":"","url":"https://api.github.com/users/srawlins","html_url":"https://github.com/srawlins","followers_url":"https://api.github.com/users/srawlins/followers","following_url":"https://api.github.com/users/srawlins/following{/other_user}","gists_url":"https://api.github.com/users/srawlins/gists{/gist_id}","starred_url":"https://api.github.com/users/srawlins/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/srawlins/subscriptions","organizations_url":"https://api.github.com/users/srawlins/orgs","repos_url":"https://api.github.com/users/srawlins/repos","events_url":"https://api.github.com/users/srawlins/events{/privacy}","received_events_url":"https://api.github.com/users/srawlins/received_events","type":"User","site_admin":false},"labels":[{"id":219928225,"node_id":"MDU6TGFiZWwyMTk5MjgyMjU=","url":"https://api.github.com/repos/dart-lang/sdk/labels/analyzer-hint","name":"analyzer-hint","color":"d4c5f9","default":false},{"id":219928186,"node_id":"MDU6TGFiZWwyMTk5MjgxODY=","url":"https://api.github.com/repos/dart-lang/sdk/labels/area-analyzer","name":"area-analyzer","color":"c7def8","default":false},{"id":1252167634,"node_id":"MDU6TGFiZWwxMjUyMTY3NjM0","url":"https://api.github.com/repos/dart-lang/sdk/labels/pkg-meta","name":"pkg-meta","color":"ffcda8","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":6,"created_at":"2019-08-29T18:02:25Z","updated_at":"2019-08-30T10:05:19Z","closed_at":null,"author_association":"MEMBER","body":"As per the static extension spec's [comments on sealed classes](https://github.com/dart-lang/language/blob/master/accepted/future-releases/static-extension-methods/feature-specification.md#sealed-classes):\r\n\r\n> If we introduce sealed classes, we may want to consider whether to allow extensions on sealed classes, since adding members even to a sealed class could still be a breaking change.\r\n\r\n> One of the reasons for having sealed classes is that it ensures the author can add to the interface without breaking code. If adding a member changes the meaning of code which currently calls an extension member, that reason is eliminated.\r\n\r\n> Since it's possible to add extensions on superclass (including Object), it would not be sufficient to disallow declaring extensions on a sealed class, you would have to disallow invoking an extension on a sealed class, at least without an explicit override (which would also prevent breaking if a similarly named instance member is added).\r\n\r\nOof. This certainly adds some complexity. We can at least do the first part (\"disallow declaring ...\") easily.\r\n\r\nCC @lrhn @bwilkerson @matanlurey @nex3  (Matan and Natalie are the primary requestors of sealed, I believe, and the Sass package uses `@sealed` a lot.)"},"comment":{"url":"https://api.github.com/repos/dart-lang/sdk/issues/comments/526544571","html_url":"https://github.com/dart-lang/sdk/issues/38107#issuecomment-526544571","issue_url":"https://api.github.com/repos/dart-lang/sdk/issues/38107","id":526544571,"node_id":"MDEyOklzc3VlQ29tbWVudDUyNjU0NDU3MQ==","user":{"login":"eernstg","id":9606994,"node_id":"MDQ6VXNlcjk2MDY5OTQ=","avatar_url":"https://avatars1.githubusercontent.com/u/9606994?v=4","gravatar_id":"","url":"https://api.github.com/users/eernstg","html_url":"https://github.com/eernstg","followers_url":"https://api.github.com/users/eernstg/followers","following_url":"https://api.github.com/users/eernstg/following{/other_user}","gists_url":"https://api.github.com/users/eernstg/gists{/gist_id}","starred_url":"https://api.github.com/users/eernstg/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/eernstg/subscriptions","organizations_url":"https://api.github.com/users/eernstg/orgs","repos_url":"https://api.github.com/users/eernstg/repos","events_url":"https://api.github.com/users/eernstg/events{/privacy}","received_events_url":"https://api.github.com/users/eernstg/received_events","type":"User","site_admin":false},"created_at":"2019-08-30T10:05:19Z","updated_at":"2019-08-30T10:05:19Z","author_association":"MEMBER","body":"@nex3 wrote:\r\n> I want to _trust_ that I'll continue to call the slice() extension method\r\n> that I wrote and tested my code against.\r\n\r\nI think we should address this kind of issue in a symmetric manner.\r\n\r\nWhether or not the receiver type is `@sealed`, we can have breakage in both directions: A new version of the package that delivers the receiver class is updated, a new instance method is added, and an extension method invocation could be captured by this new instance method. Or some update, or new dependency, or additional import could add a new extension to the current scope and change an instance method invocation to an extension method invocation. So the priority of extension methods vs. instance methods will never be able to eliminate this kind of breakage, we just have to choose in which direction we want to have the breakage.\r\n\r\nSo how about this?:\r\n\r\n> extension methods always know the interface they're extending\r\n\r\nThat wouldn't actually be true in all cases: A new version of said interface wasn't known to the authors of the extension at the time when it was written. So we can't trust extension authors to take these name clashes into account, even if there's a general agreement to try to do that.\r\n\r\nI think the priority that we have today (\"instance members always win\") is conceptually attractive, because it means that we can read code and trust existing knowledge about a given receiver type (if I know that `C` has a method `foo` then I can trust `myC.foo()` to call the instance method, no matter which and how many extensions we have in scope).\r\n\r\nWith the opposite priority we would give extension methods the special power that they can override instance methods (which may be cool and convenient in some cases), but it would mean that readers of the code would _always_ need to take extension methods into account in order to understand the effect of a given method invocation. I suspect that the number of wasted developer brain cycles will be bigger if we always need to think \"maybe it's an extension method?\" as opposed to only having to think so when the given member name is unknown in the static type of the receiver.\r\n\r\nHowever, any of these kinds of breakage could actually be handled using a symmetric non-language approach if our tooling would allow for detecting re-binding events in existing code: \"Here is `myC.foo()`, and it resolved as `MyExtension<int>(myC).foo()` in commit 38792dd, but in commit 6ffda779 it calls `C.foo`\".\r\n\r\nWe might still need to edit some code after a package version upgrade, or whatever the trigger is, but the crucial first step is to get notifications about potential bugs of this kind.\r\n\r\nI'm not saying that it is easy (what does it _mean_ to be \"the same code\" in two versions of a library?), but it might be quite usable even with a rather simplistic approach (starting with \"this library wasn't changed at all, so every piece of code in here is the same\").\r\n\r\nBased on this, I think there is no need for special rules about sealed classes."}}
{"actor":{"display_login":"eernstg"}}