8000
cleanup method as function semantic in `@list` by Guest0x0 · Pull Request #2307 · moonbitlang/core · GitHub
More Web Proxy on the site http://driver.im/
You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR reverts #2224, and fix some related issue.
We have deprecated the behavior of calling methods as functions directly. Currently, the recommended API design guideline is:
always favor method when the first parameter is Self
do not provide function alternative unless necessary
For existing library, library authors need to do nothing: downstream users will receive a deprecated warning when calling methods as function, and fix these usage. There is no need to provide fnalias for methods, as in #2224, unless necessary (for example, for math functions Double::sin etc, people are used to regular function syntax due to math notation convention).
So this PR reverts #2224, remove fnalias for @list.T methods. Downstream users of @list will receive a deprecated warning if they use @list.T methods as regular functions, and it is their responsibility to fix these usage. This is not a breaking change since it's a warning, not a fatal error.
One exception is zip. Since zip is a binary operation on list, it sometimes feels more natural to write @list.zip(xs, ys), so the fnalias for zip is kept in this PR.
Category
Correctness Code Snippet
pub fnalias T::zip Recommendation
Consider adding a deprecation warning for the function variant of other methods to help users migrate their code gradually. Reasoning
While keeping only zip as function variant is justified by its binary operation nature, removing all other function variants without a deprecation period might break existing code silently. A deprecation warning would help users identify and fix affected code.
Documentation Needs Update
Category
Maintainability Code Snippet
// Example:
/// ```mbt
/// let ls = @list.intersperse(@list.from_array(["1", "2", "3", "4", "5"]), "|")
Recommendation
Update code examples to use method syntax consistently, e.g.:
Reasoning
Some code examples still use the function call syntax that is being deprecated. Documentation should reflect the recommended style to help users write idiomatic code.
Internal Function Visibility
Category
Maintainability Code Snippet
fn[A] reverse_inplace(self : T[A]) -> T[A] Recommendation
Add explicit visibility modifier (pub/priv) to internal functions like reverse_inplace Reasoning
Function visibility should be explicitly declared to make the API boundaries clear. This helps prevent accidental usage of internal functions and makes refactoring easier.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR reverts #2224, and fix some related issue.
We have deprecated the behavior of calling methods as functions directly. Currently, the recommended API design guideline is:
SelfFor existing library, library authors need to do nothing: downstream users will receive a deprecated warning when calling methods as function, and fix these usage. There is no need to provide
fnaliasfor methods, as in #2224, unless necessary (for example, for math functionsDouble::sinetc, people are used to regular function syntax due to math notation convention).So this PR reverts #2224, remove
fnaliasfor@list.Tmethods. Downstream users of@listwill receive a deprecated warning if they use@list.Tmethods as regular functions, and it is their responsibility to fix these usage. This is not a breaking change since it's a warning, not a fatal error.One exception is
zip. Sincezipis a binary operation on list, it sometimes feels more natural to write@list.zip(xs, ys), so thefnaliasforzipis kept in this PR.cc @illusory0x0 @peter-jerry-ye