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

Do we need an MasgnNode ? #260

Open
bquorning opened this issue Mar 26, 2023 · 2 comments
Open

Do we need an MasgnNode ? #260

bquorning opened this issue Mar 26, 2023 · 2 comments

Comments

@bquorning
Copy link
Contributor

bquorning commented Mar 26, 2023

Do we need a node to help with “multi-assignment”? I came across this code in the rubocop-thread_safety gem: https://github.com/rubocop/rubocop-thread_safety/blob/2662a8142f41a53bc3bc65eb2c670db18b3c581a/lib/rubocop/cop/thread_safety/mutable_class_instance_variable.rb#L99C13-L110

def on_masgn(node)
  return unless in_class?(node)

  mlhs, values = *node
  return unless values.array_type?

  mlhs.to_a.zip(values.to_a).each do |lhs, value|
    next unless lhs.ivasgn_type?

    on_assignment(value)
  end
end

RuboCop’s InternalAffairs/NodeDestructuring cop tells me to “Use the methods provided with the node extensions instead of manually destructuring nodes” on the mlhs, values = *node line. But the RuboCop::AST::Node class doesn’t really offer any specialised helper method for this type of node.

So I thought it may be useful to add a RuboCop::AST::MasgnNode class with access to the left-hand-side list (also a RuboCop::AST::Node instance) and the right-hand-side RuboCop::AST::ArrayNode of values? And perhaps the class could also have a method to iterate over each assignment pair, much like the mlhs.to_a.zip(values.to_a).each, though perhaps it should be lazy instead of eager.

@marcandre
Copy link
Contributor

marcandre commented Mar 27, 2023

Sure, this could be a good addition, although it is tricky.

Are you up to produce a PR?

mlhs seems easiest.
It might be nice to have a shortcut to mlhs's children, but I'm not sure what's a good name.

On the right side it's a bit tricky, as it may be an expression, or may be array.

The enumerator sounds interesting, but is trickiest, as there are multiple "unhappy" cases:

  1. non array rhs (a, b = c): what does the enumerator do?
  2. lhs has less elements than rhs (shortest example: a, = b, c): what does the enumerator do?
    3) lhs has more elements than rhs seems really silly, but we have to wonder what to do in that case too.
  3. lhs has more elements than rhs (shortest example: a, b = *c): what does the enumerator do?

@dvandersluis
Copy link
Member

I had started working on nodes for masgn/mlhs a couple years ago in #203 FWIW (looks like there were some open comments that I missed). It might be a good starting point, or it can be closed.

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

3 participants