Skip to content

Print deprecation warnings on deprecated method argument#15999

Merged
straight-shoota merged 14 commits intocrystal-lang:masterfrom
ysbaddaden:feature/warning-on-deprecated-method-argument
Jul 30, 2025
Merged

Print deprecation warnings on deprecated method argument#15999
straight-shoota merged 14 commits intocrystal-lang:masterfrom
ysbaddaden:feature/warning-on-deprecated-method-argument

Conversation

@ysbaddaden
Copy link
Collaborator

We can annotate a method argument, which means we can technically put the @[Deprecated] annotation to deprecate a method argument, but there was no incentive to do it... yet.

In #15924 we want to deprecate a specific method argument (blocking) and we can't use the usual duplicated methods, one with the argument (deprecated) and another one without it, because it too many cases this is a positional argument, and it comes after other positional arguments with a default value.

This PR adds deprecation checks for method arguments, and prints a warning when an argument is deprecated. For example:

def foo(a, b = nil, @[Deprecated] c = nil)
end

foo(1)
foo(1, 2)
foo(1, 2, 3)
foo(1, c: 3)

Will generate the following deprecations, only complaining when we pass an explicit value to the deprecated argument (the first two calls are fine, the last two are not):

In x.cr:6:11

 6 | foo(1, 2, 3)
               ^
Warning: Deprecated argument c.

In x.cr:7:11

 7 | foo(1, c: 3)
               ^
Warning: Deprecated argument c.

A total of 2 warnings were found.

See the specs for all the different cases, inclusing splats, and double splats.

The PR also fixes a couple pre-requisites, for example making sure that each arg annotation is correctly copied when expanding or duplicating a def, or that the original name of an arg is also carried over, so we don't complain about argument __temp_1 being deprecated.

@Blacksmoke16
Copy link
Member

Glad to see another solid use case for annotations on parameters! Good stuff!

Copy link
Member

@straight-shoota straight-shoota left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the pedantic nagging: I'd prefer to extract some of the changes into separate PRs. Especially the first two commits with bugfixes should be merged individually.
I suppose the rest can be together as part of this feature.

@ysbaddaden
Copy link
Collaborator Author

Hum, I should add specs for .new and #initialize too, that don't follow the same path for special arg expansions (and #initialize is expanded as .new too).

@ysbaddaden
Copy link
Collaborator Author

ysbaddaden commented Jul 21, 2025

Of course, the #initialize to .new expansions only works partially (positional args are working, but named args, splats and double splats don't)

@ysbaddaden
Copy link
Collaborator Author

Now with fix from #16013 and specs for expansion of #initialize to .new default arguments.

@ysbaddaden
Copy link
Collaborator Author

I'll rebase or merge when the 3 dependent fixes are reviewed and merged: #16007, #16008 and #16013.

@ysbaddaden
Copy link
Collaborator Author

I fixed the conflicts with master.

@straight-shoota straight-shoota merged commit 40cdb59 into crystal-lang:master Jul 30, 2025
39 checks passed
@ysbaddaden ysbaddaden deleted the feature/warning-on-deprecated-method-argument branch July 31, 2025 09:21
straight-shoota added a commit that referenced this pull request Oct 11, 2025
Adds documentation for newly supported deprecation of types and aliases (#15962) and parameters (#15999).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants