search

Shopify / ruby-lsp

An opinionated language server for Ruby
https://shopify.github.io/ruby-lsp/
MIT License
1.59k stars 157 forks source link

Support Rails auto-detection for engines/railties #2750

Open rhysb27 opened 3 weeks ago

rhysb27 commented 3 weeks ago

I have checked that this feature is not already implemented

Use case

The LSP currently does not auto-detect Rails for Engine/Railtie projects, presumably as of #2218. The ability to use the Rails add-on when working on such projects would be extremely useful, particularly for tests and feature congruence when working on Rails applications and common dependencies simultaneously.

Description

A few options come to mind:

  1. The Rails LSP add-on to be automatically installed upon detection of a Railtie-based project, instead of just a concrete application
  2. Automatic inclusion upon detection of a Rails dummy application
  3. Manual configuration via VS Code Settings to "tell" Ruby LSP to install the add-on

Implementation

Ideally the auto-detection for Rails apps would be modified to add an additional check - whether or not the main class descends from Rails::Railtie. Variable-depth namespacing may add complexity to finding the main class, but I'm no expert. Applying the current check a second time at [spec|test]/*/config/application.rb may be a suitable alternative for basing inclusion on the existence of a dummy app - something included as standard by the Rails plugin generator.

If these options aren't feasible, could some mechanism be added to allow users to manually specify that the Rails add-on should be included?

andyw8 commented 3 weeks ago

Potentially related: https://github.com/Shopify/ruby-lsp-rails/issues/221

vinistock commented 2 weeks ago

Thank you for the feature suggestion! Indeed, we should provide the same features for engines. Is there anything mandatory about the structure of engines or some convention we can rely on?

For example, is it mandatory/recommended to have a lib/my_gem/engine.rb with something that inherits from Rails::Engine?

I think automated detection is the way to go here, we just need to ensure that we're accurate about detection.

andyw8 commented 2 weeks ago

The Spree gem may be good to be good example to verify against as it contains multiple engines.

rhysb27 commented 2 weeks ago

Is there anything mandatory about the structure of engines or some convention we can rely on?

For example, is it mandatory/recommended to have a lib/my_gem/engine.rb with something that inherits from Rails::Engine?

@vinistock thanks for the interest! As far as I'm aware, any engine or "simple" plugin that hooks into the Rails initialisation timeline (and thus is likely tested via a dummy app as per the plugin generator) does so via a core class which descends from Rails::Railtie. Engines add an extra layer but Engine's parent is still Railtie. Using this as an automatic detection rule seems sound, at least to me. @andyw8 rightfully points out that some more complex projects may have multiple engines/railties but as long as at least one is present, it would be unusual for the project to not benefit from the Rails add-on.

As mentioned though, plugins/engines may be arbitrarily namespaced. The standard/generated railtie location for a single-depth namespace is indeed lib/my_gem/[engine|railtie].rb, but lib/my/gem/[engine|railtie].rb would also be valid. I suspect as long as the file a) descends from Railtie and b) is exposed to the installer, the location and filepath can be completely arbitrary, but given Rail's philosophy of convention over configuration the above base cases should satisfy the majority of cases without introducing unreasonable false positives.