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

respect signature declaration style. #174

Open
aaronmallen opened this issue Dec 28, 2024 · 4 comments
Open

respect signature declaration style. #174

aaronmallen opened this issue Dec 28, 2024 · 4 comments

Comments

@aaronmallen
Copy link

aaronmallen commented Dec 28, 2024

Currently rbs inline will generate signatures that do not respect the RBS/Lint/AmbiguousOperatorPrecedence cop from rubocop-on-rbs.

Given the following declaration:

class MyClass
  # @rbs!
  #   interface _MyInterface
  #     def some_method: (untyped) -> bool
  #   end
  # 
  #   type some_type = Class | Module | Proc | (_MyInterface & Object)
end

RBS inline will generate the following signature:

class MyClass
  interface _MyInterface
    def some_method: (untyped) -> bool
  end

  type some_type = Class | Module | Proc | _MyInterface & Object # notice the lack of parentheses
end

Which violates the RBS/Lint/AmbiguousOperatorPrecedence cop. I am not suggesting RBS inline should directly support or even acknowledge the existence of the rubocop-on-rbs gem, however it would be pretty nice if it kept the signature how I wrote it so that I don't have to correct the cop violation everytime I regenerate signatures.

@aaronmallen aaronmallen changed the title respect RBS/Lint/AmbiguousOperatorPrecedence respect signature declaration style. Dec 28, 2024
@ParadoxV5
Copy link

– and I thought @rbs! copy-pastes its contents verbatim.

@tk0miya
Copy link
Contributor

tk0miya commented Dec 31, 2024

Internally, RBS::Inline goes re-formatting RBS using RBS::Writer before generating a RBS file.
The RBS declarations are copy-pasted once to the buffer, but reformatted before generating.

For example, this code will be converted to the aligned RBS:

class Hello
  # @rbs!
  #   def hello
  #   : (
  #     name: String
  #     )
  #   -> (((Integer)))
end
class Hello
  def hello: (name: String) -> Integer
end

It seems the writer dropped the parens on reformatting. I'll take a look the module later.

@tk0miya
Copy link
Contributor

tk0miya commented Jan 3, 2025

FYI: I found RBS::Types::Union#to_s drops the parens.

irb(main):001> require 'rbs'
=> true
irb(main):002> RBS::Parser.parse_method_type("() -> (String | Integer & MyMod)").to_s
=> "() -> (String | Integer & MyMod)"
irb(main):003> RBS::Parser.parse_method_type("() -> (String | (Integer & MyMod))").to_s
=> "() -> (String | Integer & MyMod)"
irb(main):004> RBS::Parser.parse_type("(String | (Integer & MyMod))").to_s
=> "String | Integer & MyMod"

tk0miya added a commit to tk0miya/rbs that referenced this issue Jan 13, 2025
At present, the result of `Union#to_s` does not respect
`RBS/Lint/AmbiguousOperatorPrecedence` cop from rubocop-on-rbs.

It would be better to wrap intersections in unions by paranthesis.

* Before: `Integer | String & bool`
* After:  `Integer | (String & bool)`

ref: soutaro/rbs-inline#174
tk0miya added a commit to tk0miya/rbs that referenced this issue Jan 13, 2025
At present, the result of `Union#to_s` does not respect
`RBS/Lint/AmbiguousOperatorPrecedence` cop from rubocop-on-rbs.

It would be better to wrap intersections in unions by paranthesis.

* Before: `Integer | String & bool`
* After:  `Integer | (String & bool)`

ref: soutaro/rbs-inline#174
@tk0miya
Copy link
Contributor

tk0miya commented Jan 13, 2025

Now I posted ruby/rbs#2231. I hope it will be merged soon.

tk0miya added a commit to tk0miya/rbs that referenced this issue Jan 13, 2025
At present, the result of `Union#to_s` does not respect
`RBS/Lint/AmbiguousOperatorPrecedence` cop from rubocop-on-rbs.

It would be better to wrap intersections in unions by paranthesis.

* Before: `Integer | String & bool`
* After:  `Integer | (String & bool)`

ref: soutaro/rbs-inline#174
tk0miya added a commit to tk0miya/rbs that referenced this issue Jan 13, 2025
At present, the result of `Union#to_s` does not respect
`RBS/Lint/AmbiguousOperatorPrecedence` cop from rubocop-on-rbs.

It would be better to wrap intersections in unions by paranthesis.

* Before: `Integer | String & bool`
* After:  `Integer | (String & bool)`

ref: soutaro/rbs-inline#174
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