-
Notifications
You must be signed in to change notification settings - Fork 144
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
Add RFC-1112 - "Obsolete allowed to use Obsolete" #617
Changes from 8 commits
8731cac
9a4c401
e35cede
0ad4ae7
af85848
c0d791d
97203a7
a1672ab
361a358
edc7bcd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,126 @@ | ||
# F# RFC FS-1112 - Obsolete allowed to use Obsolete | ||
|
||
<!--The design suggestion [Obsolete allowed to use Obsolete](https://github.com/fsharp/fslang-suggestions/issues/1055) has been marked "approved in principle". | ||
--> | ||
|
||
This RFC covers the detailed proposal for this suggestion. | ||
|
||
- [x] [Suggestion](https://github.com/fsharp/fslang-suggestions/issues/1055) | ||
- [ ] Approved in principle | ||
- [ ] [Implementation](https://github.com/dotnet/fsharp/pull/FILL-ME-IN) | ||
- [ ] Design Review Meeting(s) with @dsyme and others invitees | ||
- [Discussion](https://github.com/fsharp/fslang-design/discussions/FILL-ME-IN) | ||
|
||
# Summary | ||
|
||
The general idea is to allow types and functions marked with `System.ObsoleteAttribute` to use other `Obsolete`-marked members without | ||
This comment was marked as resolved.
Sorry, something went wrong. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If mentioning Modules separetely, then I guess i should add all combinations related to them to the Design section? |
||
having the FS0044 warning emitted. | ||
|
||
# Motivation | ||
|
||
Assume we completely redesigned API of some library. Then, to keep the backward compatibility we keep the old members and mark | ||
them with `Obsolete`. But if there is no new equivalent functions to old functions, we have to use obsolete functions | ||
and members in that old obsolete API. Since the member is already marked as `Obsolete`, and hence, unsupported, there is no reason | ||
to warn the developer of the function/type about using obsolete members in it. | ||
|
||
# Detailed design | ||
|
||
#### 1. Functions marked as Obsolete can use functions marked as Obsolete: | ||
|
||
```fs | ||
[<Obsolete>] | ||
let add a b = a + b | ||
|
||
[<Obsolete>] | ||
let func a b = | ||
add a b // no warning | ||
``` | ||
|
||
#### 2. Types marked as Obsolete can use functions marked as Obsolete | ||
|
||
```fs | ||
[<Obsolete>] | ||
let add a b = a + b | ||
|
||
[<Obsolete>] | ||
type Aaa () = | ||
static member someMember a b = | ||
add a b // no warning | ||
``` | ||
|
||
#### 3. Functions marked as Obsolete can use types marked as Obsolete | ||
|
||
```fs | ||
[<Obsolete>] | ||
type Aaa () = | ||
static member someMember a b = | ||
a + b | ||
|
||
[<Obsolete>] | ||
let add a b = | ||
Aaa.someMember a b // no warning | ||
``` | ||
|
||
#### 4. Any function can use an obsolete function or type if its enclosing function or type is Obsolete | ||
|
||
Using obsolete type in a local function: | ||
```fs | ||
[<Obsolete>] | ||
type Aaa () = | ||
static member someMember a b = | ||
a + b | ||
|
||
[<Obsolete>] | ||
let outerFunc a b = | ||
let add a b = | ||
Aaa.someMember a b // no warning because the outer function is obsolete | ||
add a b | ||
``` | ||
|
||
Using obsolete function in a local function: | ||
```fs | ||
[<Obsolete>] | ||
let someAdd a b = a + b | ||
|
||
[<Obsolete>] | ||
let outerFunc a b = | ||
let add a b = | ||
someAdd a b // no warning | ||
add a b | ||
``` | ||
|
||
#### 5. Modules | ||
|
||
Modules behave identically to types at this point. | ||
|
||
#### 6. Behaviour under different values of the `IsError` property of `System.ObsoleteAttribute` | ||
|
||
* `Obsolete(_, error=true)` can use `Obsolete(_, error=true)` | ||
* `Obsolete(_, error=true)` can use `Obsolete(_, error=false)` | ||
* `Obsolete(_, error=false)` can use `Obsolete(_, error=false)` | ||
* `Obsolete(_, error=false)` can **not** use `Obsolete(_, error=true)` (works by old rules) | ||
|
||
#### 7. New warning message | ||
|
||
New text for the warning will indicate that the outer member should be marked `Obsolete`, like we do it | ||
with `async` in C#. For example, `The construct is deprecated and should not be used. Consider marking the outer function or type as Obsolete.` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add the message from C# There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why should you expect everyone to be familiar with what C# does? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's not essential to be familiar with it to understand the new text. But those who are familar will understand the analogy. But if necessary, I can add info about the C#'s message or remove the mention of C# completely |
||
|
||
# Drawbacks | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Potential for abuse? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't think of any potential for abuse at the moment. Let me know if there's some case |
||
# Alternatives | ||
|
||
# Compatibility | ||
|
||
* Is this a breaking change? | ||
|
||
**No** | ||
* What happens when previous versions of the F# compiler encounter this design addition as source code? | ||
|
||
**It emits the warning** | ||
* What happens when previous versions of the F# compiler encounter this design addition in compiled binaries? | ||
|
||
**It emits the warning** | ||
|
||
# Unresolved questions | ||
|
||
Maybe some corner cases? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing discussion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I already should start one, since it's not yet approved