Skip to content
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

False positive positional parameter mismatch warning #12150

Closed
Blacksmoke16 opened this issue Jun 23, 2022 · 2 comments · Fixed by #12167
Closed

False positive positional parameter mismatch warning #12150

Blacksmoke16 opened this issue Jun 23, 2022 · 2 comments · Fixed by #12167
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:semantic

Comments

@Blacksmoke16
Copy link
Member

Blacksmoke16 commented Jun 23, 2022

Bug Report

While working on updating Athena to fix the warnings from #11915, I came across an unexpected warning:

module Test
  abstract def visit(values : Array(Int32))
end

class Foo
  include Test

  def visit(values : Array(Int32))
  end

  def visit(data : _)
  end
end

results in:

11 | def visit(data : _)
               ^---
Warning: positional parameter 'data' corresponds to parameter 'values' of the overridden method Test#visit(values : Array(Int32)), which has a different name and may affect named argument passing

I would have expected this to not produce a warning since its own overload and not part of the interface at all.

@Blacksmoke16 Blacksmoke16 added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:semantic labels Jun 23, 2022
@HertzDevil
Copy link
Contributor

HertzDevil commented Jun 23, 2022

I will probably add a check where if any one of the overloads implements the abstract def completely, then no warnings would be generated for any other overloads. This requires that all the parameters are identical and that all the restrictions are contravariant (less strict than the corresponding ones in the abstract def):

abstract class Foo
  abstract def foo(x : Int, y : Int)
end

class Bar1 < Foo
  def foo(x : Int, y : Int); end
  def foo(x : Int, z : Int, *xs); end # okay, no warnings
end

class Bar2 < Foo
  def foo(x : Int, y : Int, *xs); end
  def foo(x : Number, z : Int, *xs); end # warns, because first overload has different parameters
end

class Bar3 < Foo
  def foo(x : Int32, y : Int32); end
  def foo(x : Int, z : Int); end # warns, because first overload is not contravariant
end

class Bar4 < Foo
  def foo(x : Int, y : Int); end
  def foo(x : Int, z : Int); end # warns, because this overload redefines the first one
end

Otherwise, one has to determine whether an abstract def has been exhausted by previous overloads in a class:

abstract class Foo
  abstract def foo(x : Int32 | String)
end

class Bar < Foo
  def foo(x : Int32); end
  def foo(x : String); end
  # need to show that the above overloads exhaust the abstract def
  # (at this point overloads are already ordered by strictness)
  def foo(y); end
end

which amounts to solving #8232.

@HertzDevil
Copy link
Contributor

The same can also be said about return type checks: #11915 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:semantic
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants