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

Typechecking DSL files: top-level context and block context #974

Open
lloeki opened this issue Dec 6, 2023 · 3 comments
Open

Typechecking DSL files: top-level context and block context #974

lloeki opened this issue Dec 6, 2023 · 3 comments
Milestone

Comments

@lloeki
Copy link
Contributor

lloeki commented Dec 6, 2023

DSLs are great. Typed DSLs are even better: from the usual type checking of values to leveraging the LSP for autocompletion it would be fantastic that Steep could do a better job. Especially as a ton of DSLs are out there for many purposes and Ruby devs by and large love that, so I feel RBS/Steep would be incomplete without proper support of DSLs.

Here's the situation.

Steep DSL

Steep has a DSL... so I tried to typecheck Steepfile with steep!

# Steepfile
# @type self: Steep::Project::DSL

target :lib do
  check '.'
  check 'Steepfile'
  check 'Rakefile'

  library 'steep'
  library 'uri'
  library 'rake'

  signature '.'
end

Unfortunately, despite the annotation steep check Steepfile gives:

Steepfile:4:2: [error] Type `::Steep::Project::DSL` does not have method `check`
│ Diagnostic ID: Ruby::NoMethod
│
└   check '.'
    ~~~~~

Steepfile:5:2: [error] Type `::Steep::Project::DSL` does not have method `check`
│ Diagnostic ID: Ruby::NoMethod
│
└   check 'Steepfile'
    ~~~~~

Steepfile:6:2: [error] Type `::Steep::Project::DSL` does not have method `check`
│ Diagnostic ID: Ruby::NoMethod
│
└   check 'Rakefile'
    ~~~~~

Steepfile:8:2: [error] Type `::Steep::Project::DSL` does not have method `library`
│ Diagnostic ID: Ruby::NoMethod
│
└   library 'steep'
    ~~~~~~~

Steepfile:9:2: [error] Type `::Steep::Project::DSL` does not have method `library`
│ Diagnostic ID: Ruby::NoMethod
│
└   library 'uri'
    ~~~~~~~

Steepfile:10:2: [error] Type `::Steep::Project::DSL` does not have method `library`
│ Diagnostic ID: Ruby::NoMethod
│
└   library 'rake'
    ~~~~~~~

Steepfile:12:2: [error] Type `::Steep::Project::DSL` does not have method `signature`
│ Diagnostic ID: Ruby::NoMethod
│
└   signature '.'
    ~~~~~~~~~

Steepfile:3:0: [error] Type `::Object` does not have method `target`
│ Diagnostic ID: Ruby::NoMethod
│
└ target :lib do
  ~~~~~~

Detected 8 problems from 1 file

Note that for the last one it tried to look things up in ::Object

I tried adding more annotation to the block:

# @type self: Steep::Project::DSL

target :lib do
  # @type self: Steep::Project::DSL::Target

  check '.'
  check 'Steepfile'
  check 'Rakefile'

  library 'steep'
  library 'uri'
  library 'rake'

  signature '.'
end

But it failed as well, in exactly the same way:

Steepfile:6:2: [error] Type `::Steep::Project::DSL` does not have method `check`
│ Diagnostic ID: Ruby::NoMethod
│
└   check '.'
    ~~~~~

Steepfile:7:2: [error] Type `::Steep::Project::DSL` does not have method `check`
│ Diagnostic ID: Ruby::NoMethod
│
└   check 'Steepfile'
    ~~~~~

Steepfile:8:2: [error] Type `::Steep::Project::DSL` does not have method `check`
│ Diagnostic ID: Ruby::NoMethod
│
└   check 'Rakefile'
    ~~~~~

Steepfile:10:2: [error] Type `::Steep::Project::DSL` does not have method `library`
│ Diagnostic ID: Ruby::NoMethod
│
└   library 'steep'
    ~~~~~~~

Steepfile:11:2: [error] Type `::Steep::Project::DSL` does not have method `library`
│ Diagnostic ID: Ruby::NoMethod
│
└   library 'uri'
    ~~~~~~~

Steepfile:12:2: [error] Type `::Steep::Project::DSL` does not have method `library`
│ Diagnostic ID: Ruby::NoMethod
│
└   library 'rake'
    ~~~~~~~

Steepfile:14:2: [error] Type `::Steep::Project::DSL` does not have method `signature`
│ Diagnostic ID: Ruby::NoMethod
│
└   signature '.'
    ~~~~~~~~~

Steepfile:3:0: [error] Type `::Object` does not have method `target`
│ Diagnostic ID: Ruby::NoMethod
│
└ target :lib do
  ~~~~~~

Detected 8 problems from 1 file

Note that it still references:

  • ::Object instead of ::Steep::Project::DSL for the last one
  • ::Steep::Project::DSL instead of ::Steep::Project::DSL::Target for the last one

Maybe I'm misunderstanding the usage of the @type: self annotation?

I also believe that once target is identified, Steep should be able to recognise that the block is to be evaluated in a ::Steep::Project::DSL::Target context as it is instance_eval'd, and thus ultimately find check, library, and whatnot.

Maybe Steep can figure it out all by itself which would be ideal, or it needs the help of a Steep annotation, or RBS needs to provide that information in some way?

Rake DSL

Another popular DSL, I also tried to typecheck Rakefiles:

# Rakefile
# @type self: Rake::TaskLib

namespace :foo do
  task :foo do
    puts 'foo'
  end
end

task :bar do
  puts 'bar'
end

In spite of the @type self: Rake::TaskLib annotation, steep check Rakefile gives:

Rakefile:4:2: [error] Type `::Object` does not have method `task`
│ Diagnostic ID: Ruby::NoMethod
│
└   task :foo do
    ~~~~

Rakefile:3:0: [error] Type `::Object` does not have method `namespace`
│ Diagnostic ID: Ruby::NoMethod
│
└ namespace :foo do
  ~~~~~~~~~

Rakefile:9:0: [error] Type `::Object` does not have method `task`
│ Diagnostic ID: Ruby::NoMethod
│
└ task :bar do
  ~~~~

Detected 3 problems from 1 file

Again it failed in two areas:

  • ::Object instead of Rake::TaskLib at the top level
  • ::Object instead of Rake::TaskLib on the nested block

I also believe that once top-level namespace is identified, Steep should be able to recognise that the block is still to be evaluated in a Rake::TaskLib context as it is merely yielded, and thus ultimately find nested namespace, task, and whatnot.

Again, maybe Steep can figure it out all by itself which would be ideal, or it needs the help of a Steep annotation, or RBS needs to provide that information in some way?

Gemfile

I tried type checking Gemfile too. It's a bit trickier as Bundler/Rubygems has no RBS signatures but a quick hack shows that it suffers from the same class of issues with source, gem, group :foo do .. end, etc... e.g:

Gemfile:1:0: [error] Type `::Object` does not have method `source`
│ Diagnostic ID: Ruby::NoMethod
│
└ source 'https://rubygems.org'
  ~~~~~~

Minitest::Spec

Let's say I have something trivial:

# foo.rb
class Foo
end
# foo.rbs
class Foo
end

And the corresponding spec:

# foo_spec.rb
require 'minitest/autorun'
require 'foo'

describe Foo do
  let(:foo) { 'foo' }

  describe 'bar' do
    it 'foo' do

    end
  end
end

steep check foo_spec.rb gives:

foo_spec.rb:5:2: [error] Type `::Object` does not have method `let`
│ Diagnostic ID: Ruby::NoMethod
│
└   let(:foo) { 'foo' }
    ~~~

foo_spec.rb:8:4: [error] Type `::Object` does not have method `it`
│ Diagnostic ID: Ruby::NoMethod
│
└     it 'foo' do
      ~~

Detected 2 problems from 1 file

IIUC it works better because Minitest::Spec defines describe on Object, but you can see how it fails to pick up the block evaluation context, as with Steep and Rake.

If I change it to add an annotation:

require 'minitest/autorun'
require 'foo'

describe Foo do
  # @type self: Minitest::Spec::DSL

  let(:foo) { 'foo' }

  describe 'bar' do
    it 'foo' do

    end
  end
end

It then passes.

Still, I believe it should be really nice of Steep to find the evaluation context out, as the block is called with class_eval.

Sure there's a bit of dancing with finding that dynamic cls but in terms of type I think it's all trackable in the code. Basically each class inherits from its parent in the stack, up to Minitest::Spec (and its DSL).

In addition, Steep might be able to figure out that in the following example bar is available for the second and third describe block but not the first:

require 'minitest/autorun'
require 'foo'

describe Foo do
  let(:foo) { 'foo' }

  describe 'bar' do
    def bar
      'bar'
    end

    it 'bar' do
      bar
    end

    describe 'baz' do
      it 'baz' do
        bar
      end
    end
  end

  it 'foo' do
    bar
  end
end

While currently it yells at all of them:

foo_test.rb:5:2: [error] Type `::Object` does not have method `let`
│ Diagnostic ID: Ruby::NoMethod
│
└   let(:foo) { 'foo' }
    ~~~

foo_test.rb:13:6: [error] Type `::Object` does not have method `bar`
│ Diagnostic ID: Ruby::NoMethod
│
└       bar
        ~~~

foo_test.rb:12:4: [error] Type `::Object` does not have method `it`
│ Diagnostic ID: Ruby::NoMethod
│
└     it 'bar' do
      ~~

foo_test.rb:18:8: [error] Type `::Object` does not have method `bar`
│ Diagnostic ID: Ruby::NoMethod
│
└         bar
          ~~~

foo_test.rb:17:6: [error] Type `::Object` does not have method `it`
│ Diagnostic ID: Ruby::NoMethod
│
└       it 'baz' do
        ~~

foo_test.rb:24:4: [error] Type `::Object` does not have method `bar`
│ Diagnostic ID: Ruby::NoMethod
│
└     bar
      ~~~

foo_test.rb:23:2: [error] Type `::Object` does not have method `it`
│ Diagnostic ID: Ruby::NoMethod
│
└   it 'foo' do
    ~~

Detected 7 problems from 1 file

Similar examples could probably be produced by defining bar in a module and including it or extending with it at the second describe level.

Again, maybe Steep can figure it out all by itself which would be ideal, or it needs the help of a Steep annotation, or RBS needs to provide that information in some way?

Rspec

I have not tried Rspec but the rationale should be similar as with Minitest::Spec.

Additional bits

Here's the RBS collection I used, checked out from 20e6e0f0685139dbd29df50e03367e222aa5d1b8. I ignored a couple of inconsequential gems because they were yelling too loud, muddying the output.

# rbs_collection.yaml
sources:
  - type: git
    name: ruby/gem_rbs_collection
    remote: https://github.com/ruby/gem_rbs_collection.git
    revision: main
    repo_dir: gems

path: .gem_rbs_collection

gems:
  - name: concurrent-ruby
    ignore: true
  - name: parser
    ignore: true

And for reference, bundle show:

Gems included by the bundle:
  * abbrev (0.1.1)
  * activesupport (7.1.2)
  * ast (2.4.2)
  * base64 (0.2.0)
  * bigdecimal (3.1.4)
  * bundler (2.3.26)
  * concurrent-ruby (1.2.2)
  * connection_pool (2.4.1)
  * csv (3.2.8)
  * drb (2.2.0)
  * ffi (1.16.3)
  * fileutils (1.7.2)
  * i18n (1.14.1)
  * json (2.7.1)
  * language_server-protocol (3.17.0.3)
  * listen (3.8.0)
  * logger (1.6.0)
  * minitest (5.20.0)
  * mutex_m (0.2.0)
  * parser (3.2.2.4)
  * racc (1.7.3)
  * rainbow (3.1.1)
  * rake (13.1.0)
  * rb-fsevent (0.11.2)
  * rb-inotify (0.10.1)
  * rbs (3.3.2)
  * ruby2_keywords (0.0.5)
  * securerandom (0.3.0)
  * steep (1.6.0)
  * strscan (3.0.7)
  * terminal-table (3.0.2)
  * tzinfo (2.0.6)
  * unicode-display_width (2.5.0)
@stevegeek
Copy link

Hey @lloeki I believe there is a way in RBS to define what self should be in a block (eg if it is instance_eval'd) https://github.com/ruby/rbs/blob/master/docs/syntax.md#self-type-binding

Not sure about everything in this ticket but for the Steepfile , I think the DSL methods are actually on TargetDSL

# @type self: Steep::Project::DSL::TargetDSL

@Aesthetikx
Copy link

Not sure if this is the right issue for this, but it is especially relevant I think for Gemfile because Gemfile is explicitly listed as something that you might want to check in the default Steepfile as generated by steep init: https://github.com/soutaro/steep/blob/master/lib/steep/drivers/init.rb, which as you mentioned does not seem to work out of the box.

@soutaro soutaro added this to the Steep 1.7 milestone May 8, 2024
@soutaro
Copy link
Owner

soutaro commented May 8, 2024

Confirmed that the top-level # @type self annotation is not working.

  def test_self_type_annotation
    run_type_check_test(
      signatures: {},
      code: {
        "a.rb" => <<~RUBY
          # @type self: String
          self + ""
        RUBY
      },
      expectations: <<~YAML
        ---
        - file: a.rb
          diagnostics: []
      YAML
    )
  end
  1) Failure:
TypeCheckTest#test_self_type_annotation [test/type_check_test.rb:53]:
--- expected
+++ actual
@@ -1 +1,14 @@
-""
+"---
+- file: a.rb
+  diagnostics:
+  - range:
+      start:
+        line: 3
+        character: 5
+      end:
+        line: 3
+        character: 6
+    severity: ERROR
+    message: Type `::Object` does not have method `+`
+    code: Ruby::NoMethod
+"

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

4 participants