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

Compiler/Line Number Metadata #57

Closed
wied03 opened this issue Mar 10, 2016 · 4 comments
Closed

Compiler/Line Number Metadata #57

wied03 opened this issue Mar 10, 2016 · 4 comments

Comments

@wied03
Copy link
Contributor

wied03 commented Mar 10, 2016

Add an optional compiler helper that will add_special call nodes for describe, context, it, etc.

The modified code will still call describe, call, etc. but it will add a caller metadata attribute in implicitly to the arguments list. Caller should be an array with 1 element containing the filename and line number of the spec.

Will need to handle cases where:

  • User has already supplied the caller
  • Existing metadata being passed as a hash variable
  • Existing metadata passed with { }
  • Existing metadata passed with an implicit hash
  • No metadata being passed
  • No arguments at all (not sure if describe;do;end is valid)

It should be bootstrap compatible and configurable as to when this runs, some ideas:

  • Only files ending in _spec.rb
  • Use a compiler setting (we do have access to the compiler object in the call node, we can add an additional setting)
  • Opt in (need to require the compiler patch, this one is not mutually exclusive)

Testing:

  • Unit test with MRI - see note below
  • Unit test with bootstrap/Opal compilation
  • Maybe include in RSpec's specs so they can find location? It might be too difficult to test this so it might be easier to just have a separate integration test

Compiler test, the compiled version with the patch on:

describe 'foo' do
end

should equal this with the patch off:

describe 'foo', caller: ['/some/file/path.rb:123'] do
end
@wied03 wied03 added this to the 0.7 Release milestone Mar 10, 2016
@wied03
Copy link
Contributor Author

wied03 commented Mar 10, 2016

@elia - Sound interesting? I think it could start as a monkey patch but maybe the compiler could add a framework some day.

@iliabylich
Copy link
Contributor

Does opal-rspec support latest opal master? If it does you can do it with a custom AST rewriter:

class RSpecMetadataRewriter < Opal::Rewriters::Base
  RSPEC_METHODS = %i(describe context it)

  def on_send(node)
    node = super

    recv, meth, *args = *node

    if RSPEC_METHODS.include?(meth)
      file = node.loc.expression.source_buffer.name
      line = node.loc.line
      column = node.loc.column

      loc_metadata = "#{file}:#{line}:#{column}"
      loc_metadata_pair = s(:pair, s(:sym, :loc), s(:str, loc_metadata))

      if args.last.type == :iter
        *args, iter = *args
      end

      if args.last.type == :hash
        *args, last_hash_arg = *args
      else
        last_hash_arg = s(:hash)
      end

      pairs = *last_hash_arg

      pairs += [loc_metadata_pair]
      last_hash_arg = last_hash_arg.updated(nil, pairs)


      args = [*args, last_hash_arg, iter].compact

      node.updated(nil, [recv, meth, *args])
    else
      node
    end
  end
end

Opal::Rewriter::LIST << RSpecMetadataRewriter

test.rb:

def describe(*args)
  puts "describe"
  puts args
  yield
end

def context(*args)
  puts "context"
  puts args
  yield
end

def it(*args)
  puts "it"
  puts args
end

describe 'MyClass1', type: :some_type do
  context 'when something happens' do
    it 'does the following'
  end
end
➜  opal git:(master) ✗ opal test.rb
describe
["MyClass1", {"type"=>"some_type", "loc"=>"test:18:0"}]
context
["when something happens", {"loc"=>"test:19:2"}]
it
["does the following", {"loc"=>"test:20:4"}]

➜  opal git:(master) ✗ opal ../opal/test.rb
describe
["MyClass1", {"type"=>"some_type", "loc"=>"../opal/test:18:0"}]
context
["when something happens", {"loc"=>"../opal/test:19:2"}]
it
["does the following", {"loc"=>"../opal/test:20:4"}]

@wied03
Copy link
Contributor Author

wied03 commented Aug 30, 2016

It did but it's currently broken (Sprockets is the first reason).

@elia
Copy link
Member

elia commented Nov 9, 2018

I'm inclined to rely on source-maps for browser and possibly for node too.

@elia elia closed this as completed Nov 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants