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

Ruby 2.7 warnings #672

Closed
hwo411 opened this issue Mar 27, 2020 · 11 comments
Closed

Ruby 2.7 warnings #672

hwo411 opened this issue Mar 27, 2020 · 11 comments

Comments

@hwo411
Copy link

hwo411 commented Mar 27, 2020

Describe the bug
Right now having after_enter method with keyword generates a warning with Ruby 2.7:
``usr/local/bundle/ruby/2.7.0/gems/aasm-5.0.8/lib/aasm/core/invokers/literal_invoker.rb:32: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call`

To Reproduce

class Buggy
  include AASM

  aasm do
    state :not_buggy, initial: true
    state :buggy, after_enter: :display_bug

    event :bug do
      transitions from: :not_buggy, to: :buggy
    end
  end

  def display_bug(arg:)
  end
end

Buggy.new.bug!(arg: 123)

I didn't test this code, this is an extraction from our working code.

Expected behavior
No Ruby 2.7 warnings are shown.

@anilmaurya
Copy link
Member

@the-spectator Interested in resolving this issue ?

@hwo411
Copy link
Author

hwo411 commented Mar 27, 2020

I also discovered this topic: https://bugs.ruby-lang.org/issues/16463
Might be relevant. (I discovered similar problem with just using delegate:).

@the-spectator
Copy link
Contributor

@anilmaurya Yes, I will definitely try to resolve the issues.

@anilmaurya
Copy link
Member

Hi @hwo411

I checked this issue. You should not use keyword argument in display_bug method
To solve your issue, update your code like this

class Buggy
  include AASM

  aasm do
    state :not_buggy, initial: true
    state :buggy, after_enter: :display_bug

    event :bug do
      transitions from: :not_buggy, to: :buggy
    end
  end

  def display_bug(args)
    # Now use args[:arg]
  end
end

Buggy.new.bug!(arg: 123)

Closing this issue, please reopen if it does not solve your problem.

@hwo411
Copy link
Author

hwo411 commented Apr 30, 2020

@anilmaurya does this mean that aasm won't support keyword arguments for transitions? It's worth adding to the documentation then.

I'm not familiar with aasm code, but brief check showed me that **kwargs are never passed for callbacks and adding them might solve the issue in way that it works properly for my example. Is it something that can still be considered?

@anilmaurya
Copy link
Member

@hwo411
Yes, we need to update docs mentioning not to use keywords argument.

Currently AASM support array of arguments, using hash & keyword argument together will throw this warning in 2.7.
Later on in 3.0 it will start throwing error therefore we should not allow it.

@tisba
Copy link
Contributor

tisba commented Mar 15, 2021

@anilmaurya Since I just stumbled across this, currently the README actually recommends using keyword arguments (https://github.com/aasm/aasm#parameters):
CleanShot 2021-03-15 at 15 20 21

@anilmaurya
Copy link
Member

Thank you @tisba for pointing it out, I have corrected it.

@Flixt
Copy link

Flixt commented Nov 17, 2021

Just to get this clarified: Hash/Keyword arguments, like in the example below, in guards, callbacks etc. are currently not supported, correct? (although they just work in Ruby <= 2.6)

class Job
  include AASM

  aasm do
    state :idle
    state :running

    event :run do
      transitions from: :idle, to: :running do
        after do |reason:|
          self.reason = reason
          puts "reason was #{reason}"
        end
      end
    end
  end
end

job = Job.new
job.run(reason: 'my-reason')

# => Ruby 2.6 just works
# => Ruby 2.7 prints deprecation warning as described in this issue
# => Ruby 3.0 raises "missing keyword: :reason (ArgumentError)"

Out of curiosity, and because I personally like the keyword style much more: Will it stay this way? Is there a technical reason for that? Or is it "just" a hell lot of work? I would be happy to help to get this working if it is something the maintainers would consider valuable. I'd even say it is something, that should be supported in a way newer Ruby version would expect it to work. There is no way, I as a reader of that code, could know this would blow up on newer Rubies.

LeSim added a commit to demarches-simplifiees/demarches-simplifiees.fr that referenced this issue Nov 26, 2021
LeSim added a commit to demarches-simplifiees/demarches-simplifiees.fr that referenced this issue Nov 26, 2021
LeSim added a commit to demarches-simplifiees/demarches-simplifiees.fr that referenced this issue Nov 26, 2021
LeSim added a commit to demarches-simplifiees/demarches-simplifiees.fr that referenced this issue Nov 26, 2021
LeSim added a commit to demarches-simplifiees/demarches-simplifiees.fr that referenced this issue Nov 26, 2021
LeSim added a commit to demarches-simplifiees/demarches-simplifiees.fr that referenced this issue Nov 26, 2021
@tchak
Copy link

tchak commented Nov 27, 2021

I also don't understand why it is closed as a "won't fix". Would it be possible to provide a bit more details on why?

LeSim added a commit to demarches-simplifiees/demarches-simplifiees.fr that referenced this issue Nov 29, 2021
LeSim added a commit to demarches-simplifiees/demarches-simplifiees.fr that referenced this issue Nov 30, 2021
mfo pushed a commit to demarches-simplifiees/demarches-simplifiees.fr that referenced this issue Nov 30, 2021
@Rmpanga
Copy link

Rmpanga commented Jul 21, 2022

@tchak and @Flixt This is a Ruby 2.7 and Ruby 3.0+ issue. They deprecated (in 2.7) and removed (in 3.0+) the use of keyword arguments.

Source: https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants