Support generating DSL RBI by path #968

jeffcarbs · 2022-06-03T20:45:29Z

Motivation

We often want to generate RBI for every model, worker, etc inside a namespace. Currently the only way to do that involves crawling ObjectSpace in a console or doing grep/sed magic to get the set of constants. We wanted an easier way to do this.

Another future-facing use-case is being able to run tapioca dsl --verify some/specific/path.

Sidenote: At the moment, --verify fails unless it's called with no arguments since it thinks there's stale RBI, but that's something that can be fixed separately.

Implementation

I thought it would be most ergonomic to be able to pass directory/filenames as args to the tapioca dsl but currently it's assumed that any non-flag args passed will be class names. Luckily, ruby class names and file names follow disjoint patterns:

class names must start with a capital letter or colon
path names typically (99.9% of path names I've ever seen) start with a lower case letter or slash.

Given that, we use that heuristic to split the passed args into either class names or paths and then treat them appropriately downstream. We then use Tapioca::Static::SymbolLoader.symbols_from_paths to get all the class names from the given paths.

Wanted to call out a slight difference in the error handling for paths. tapioca dsl SomeClass SomeOtherClass currently fails if none of those classes have processable DSL. This makes sense since you're specifically giving classes for which you want RBI generated. For paths, on the other hand you may not know if there's RBI to generate and I think that OK so we actually exist successfully if you've given paths but there's no constants to process. The one exception is if you've given use no paths that actually exist, in which case we'll exit with an error which should prevent typos like tapioca dsl app/mode.

Tests

See tests

Morriar

I like where this is going ❤️

Morriar · 2022-06-06T18:39:42Z

lib/tapioca/commands/dsl.rb

+      sig { params(paths: T::Array[String]).returns(T::Array[String]) }
+      def contants_from_paths(paths)
+        output = T.unsafe(self).sorbet("--no-config", "--quiet", "--print=symbol-table-json", *paths).out
+        constants_from_symbol_table_json(JSON.parse(output))
+      end
+
+      sig { params(symbol_table_json: T::Hash[String, T.untyped], prefix: T.nilable(String)).returns(T::Array[String]) }
+      def constants_from_symbol_table_json(symbol_table_json, prefix = nil)
+        kind = symbol_table_json.fetch("kind")
+        name = symbol_table_json.fetch("name")
+        return [] unless kind == "CLASS_OR_MODULE" && name.fetch("kind") == "CONSTANT"
+
+        name = name.fetch("name")
+        children = symbol_table_json.fetch("children", [])
+
+        prefixed_name = name == "<root>" ? prefix : [prefix, name].compact.join("::")
+        [prefixed_name, *children.flat_map { |c| constants_from_symbol_table_json(c, prefixed_name) }].compact
+      end
+


I think this logic should be moved to https://github.com/Shopify/tapioca/blob/main/lib/tapioca/static/symbol_loader.rb.

Good call. I actually read through that code a bit more and realized it was going exactly what I was trying to do so we can just switch to using it directly. Will push up a commit shortly.

spec/tapioca/cli/dsl_spec.rb

lib/tapioca/commands/dsl.rb

Morriar

There are a few conflicts with main and #968 (comment) hasn't been addressed.

paracycle

I am still not super convinced about passing a mix of constants/paths on the command line, but I think I would like us to see how that works in the wild first.

I have a few comments on top of the earlier ones from @Morriar, otherwise, this looks good to me, thank you.

paracycle · 2022-07-18T17:34:53Z

lib/tapioca/static/symbol_loader.rb

@@ -35,6 +35,11 @@ def gem_symbols(gem)
          symbols_from_paths(gem.files)
        end

+        sig { params(paths: T::Array[String]).returns(T::Set[String]) }
+        def dsl_symbols(paths)


Sorry about the backwards/forwards here but can we expose the symbols_from_paths as a public method instead of adding a dsl_symbols method. I have plans to use the same functionality in another context, so I think being able to get a list of symbols that correspond to a list of files is good functionality.

lib/tapioca/commands/dsl.rb

spec/tapioca/cli/dsl_spec.rb

jeffcarbs · 2023-01-17T21:45:44Z

@paracycle @Morriar - I'm picking this PR back up. I rebased from the latest main branch and updated some things based on the conversation above.

Regarding whether to exit with an error/success, I think I wound up a nice middle-ground:

if you gave us paths that actually exist and there just happen to be no processable constants, that will exit successfully
- This is necessary to support running this as a pre-commit hook which would just pass the paths to any files that have changed. It's possible/likely that the files given just happen to not have any constants to process and that shouldn't be an error.
if you didn't give us at least one valid path and there are no processable constants, that will exit with error.
- This should hopefully be enough to catch typos where you gave us a non-existent path (e.g. a typo).

jeffcarbs force-pushed the dsl-by-path branch from b3f9ead to 7e8c0b0 Compare June 3, 2022 21:23

Morriar reviewed Jun 6, 2022

View reviewed changes

Morriar reviewed Jun 7, 2022

View reviewed changes

lib/tapioca/commands/dsl.rb Show resolved Hide resolved

Morriar mentioned this pull request Jun 13, 2022

Clean helpers #986

Merged

jeffcarbs requested a review from a team as a code owner July 14, 2022 18:33

Morriar reviewed Jul 15, 2022

View reviewed changes

paracycle reviewed Jul 18, 2022

View reviewed changes

Make symbols_from_paths public

b9a7daa

jeffcarbs force-pushed the dsl-by-path branch from b973cbc to 250d376 Compare January 17, 2023 21:31

Support generating RBI by path

fe1ce98

jeffcarbs force-pushed the dsl-by-path branch from 250d376 to fe1ce98 Compare January 17, 2023 21:45

paracycle approved these changes Feb 17, 2023

View reviewed changes

paracycle merged commit c29e582 into Shopify:main Feb 17, 2023

paracycle added the feature label Feb 19, 2023

shopify-shipit bot temporarily deployed to production February 21, 2023 12:01 Inactive

jeffcarbs deleted the dsl-by-path branch February 23, 2023 18:27

This was referenced Feb 24, 2023

Avoid raising when constantizing from paths #1414

Merged

Add ability to generate DSL RBIs for classes in a given file #832

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support generating DSL RBI by path #968

Support generating DSL RBI by path #968

jeffcarbs commented Jun 3, 2022 •

edited

Loading

Morriar left a comment

Morriar Jun 6, 2022

jeffcarbs Jun 6, 2022

Morriar left a comment

paracycle left a comment

paracycle Jul 18, 2022

jeffcarbs commented Jan 17, 2023

Support generating DSL RBI by path #968

Support generating DSL RBI by path #968

Conversation

jeffcarbs commented Jun 3, 2022 • edited Loading

Motivation

Implementation

Tests

Morriar left a comment

Choose a reason for hiding this comment

Morriar Jun 6, 2022

Choose a reason for hiding this comment

jeffcarbs Jun 6, 2022

Choose a reason for hiding this comment

Morriar left a comment

Choose a reason for hiding this comment

paracycle left a comment

Choose a reason for hiding this comment

paracycle Jul 18, 2022

Choose a reason for hiding this comment

jeffcarbs commented Jan 17, 2023

jeffcarbs commented Jun 3, 2022 •

edited

Loading