diff --git a/ast/Trees.cc b/ast/Trees.cc index 53f4b5525..46bb89e97 100644 --- a/ast/Trees.cc +++ b/ast/Trees.cc @@ -1343,11 +1343,20 @@ core::NameRef Literal::asString() const { core::NameRef Literal::asSymbol() const { ENFORCE(isSymbol()); + return asName(); +} + +core::NameRef Literal::asName() const { + ENFORCE(isName()); auto t = core::cast_type_nonnull(value); core::NameRef res = t.asName(); return res; } +bool Literal::isName() const { + return isString() || isSymbol(); +} + bool Literal::isSymbol() const { return core::isa_type(value) && core::cast_type_nonnull(value).literalKind == diff --git a/ast/Trees.h b/ast/Trees.h index ce60f2695..e3012a4f9 100644 --- a/ast/Trees.h +++ b/ast/Trees.h @@ -1040,6 +1040,8 @@ EXPRESSION(Literal) { std::string toStringWithTabs(const core::GlobalState &gs, int tabs = 0) const; std::string showRaw(const core::GlobalState &gs, int tabs = 0) const; + bool isName() const; + core::NameRef asName() const; std::string nodeName() const; bool isString() const; bool isSymbol() const; diff --git a/rewriter/Minitest.cc b/rewriter/Minitest.cc index 102e71acd..0a4b1b04c 100644 --- a/rewriter/Minitest.cc +++ b/rewriter/Minitest.cc @@ -124,30 +124,42 @@ core::LocOffsets declLocForSendWithBlock(const ast::Send &send) { } // namespace -ast::ExpressionPtr recurse(core::MutableContext ctx, bool isClass, ast::ExpressionPtr body); +ast::ExpressionPtr recurse(core::MutableContext ctx, bool isClass, ast::ExpressionPtr body, bool insideDescribe); -ast::ExpressionPtr prepareBody(core::MutableContext ctx, bool isClass, ast::ExpressionPtr body) { - body = recurse(ctx, isClass, std::move(body)); +ast::ExpressionPtr prepareBody(core::MutableContext ctx, bool isClass, ast::ExpressionPtr body, bool insideDescribe) { + body = recurse(ctx, isClass, std::move(body), insideDescribe); if (auto bodySeq = ast::cast_tree(body)) { for (auto &exp : bodySeq->stats) { - exp = recurse(ctx, isClass, std::move(exp)); + exp = recurse(ctx, isClass, std::move(exp), insideDescribe); } - bodySeq->expr = recurse(ctx, isClass, std::move(bodySeq->expr)); + bodySeq->expr = recurse(ctx, isClass, std::move(bodySeq->expr), insideDescribe); } return body; } +// Namer only looks at ancestors at the ClassDef top-level. If a `describe` block has ancestor items +// at the top level inside the InsSeq of the Block body, that should count as a an ancestor in namer. +// But if we just plop the whole InsSeq as a single element inside the the ClassDef rhs, they won't +// be at the top level anymore. +ast::ClassDef::RHS_store flattenDescribeBody(ast::ExpressionPtr body) { + ast::ClassDef::RHS_store rhs; + if (auto bodySeq = ast::cast_tree(body)) { + absl::c_move(bodySeq->stats, back_inserter(rhs)); + rhs.emplace_back(move(bodySeq->expr)); + } else { + rhs.emplace_back(move(body)); + } + + return rhs; +} + string to_s(core::Context ctx, ast::ExpressionPtr &arg) { auto argLit = ast::cast_tree(arg); string argString; - if (argLit != nullptr) { - if (argLit->isString()) { - return argLit->asString().show(ctx); - } else if (argLit->isSymbol()) { - return argLit->asSymbol().show(ctx); - } + if (argLit != nullptr && argLit->isName()) { + return argLit->asName().show(ctx); } auto argConstant = ast::cast_tree(arg); if (argConstant != nullptr) { @@ -195,21 +207,23 @@ ast::ExpressionPtr getIteratee(ast::ExpressionPtr &exp) { } ast::ExpressionPtr prepareTestEachBody(core::MutableContext ctx, core::NameRef eachName, ast::ExpressionPtr body, - ast::MethodDef::ARGS_store &args, ast::InsSeq::STATS_store destructuringStmts, - ast::ExpressionPtr &iteratee); + const ast::MethodDef::ARGS_store &args, + absl::Span destructuringStmts, + ast::ExpressionPtr &iteratee, bool insideDescribe); // this applies to each statement contained within a `test_each`: if it's an `it`-block, then convert it appropriately, // otherwise flag an error about it ast::ExpressionPtr runUnderEach(core::MutableContext ctx, core::NameRef eachName, - ast::InsSeq::STATS_store &destructuringStmts, ast::ExpressionPtr stmt, - ast::MethodDef::ARGS_store &args, ast::ExpressionPtr &iteratee) { + absl::Span destructuringStmts, ast::ExpressionPtr stmt, + const ast::MethodDef::ARGS_store &args, ast::ExpressionPtr &iteratee, + bool insideDescribe) { // this statement must be a send if (auto *send = ast::cast_tree(stmt)) { + auto correctBlockArity = send->hasBlock() && send->block()->args.size() == 0; // the send must be a call to `it` with a single argument (the test name) and a block with no arguments - if ((send->fun == core::Names::it() && send->numPosArgs() == 1 && send->hasBlock() && - send->block()->args.size() == 0) || + if ((send->fun == core::Names::it() && send->numPosArgs() == 1 && correctBlockArity) || ((send->fun == core::Names::before() || send->fun == core::Names::after()) && send->numPosArgs() == 0 && - send->hasBlock() && send->block()->args.size() == 0)) { + correctBlockArity)) { core::NameRef name; auto arg0Loc = core::LocOffsets::none(); if (send->fun == core::Names::before()) { @@ -226,7 +240,20 @@ ast::ExpressionPtr runUnderEach(core::MutableContext ctx, core::NameRef eachName // pull constants out of the block ConstantMover constantMover; ast::ExpressionPtr body = move(send->block()->body); - ast::TreeWalk::apply(ctx, constantMover, body); + + // we don't need to make a new body if the original one was empty + if (!ast::isa_tree(body)) { + ast::TreeWalk::apply(ctx, constantMover, body); + + // add the destructuring statements to the block if they're present + if (!destructuringStmts.empty()) { + ast::InsSeq::STATS_store stmts; + for (auto &stmt : destructuringStmts) { + stmts.emplace_back(stmt.deepCopy()); + } + body = ast::MK::InsSeq(body.loc(), std::move(stmts), std::move(body)); + } + } // pull the arg and the iteratee in and synthesize `iterate.each { |arg| body }` ast::MethodDef::ARGS_store new_args; @@ -234,15 +261,6 @@ ast::ExpressionPtr runUnderEach(core::MutableContext ctx, core::NameRef eachName new_args.emplace_back(arg.deepCopy()); } - // add the destructuring statements to the block if they're present - if (!destructuringStmts.empty()) { - ast::InsSeq::STATS_store stmts; - for (auto &stmt : destructuringStmts) { - stmts.emplace_back(stmt.deepCopy()); - } - body = ast::MK::InsSeq(body.loc(), std::move(stmts), std::move(body)); - } - auto blk = ast::MK::Block(send->block()->loc, move(body), std::move(new_args)); auto each = ast::MK::Send0Block(send->loc, iteratee.deepCopy(), core::Names::each(), send->loc.copyWithZeroLength(), move(blk)); @@ -251,10 +269,19 @@ ast::ExpressionPtr runUnderEach(core::MutableContext ctx, core::NameRef eachName auto method = addSigVoid(ast::MK::SyntheticMethod0(send->loc, declLoc, arg0Loc, move(name), move(each))); // add back any moved constants return constantMover.addConstantsToExpression(send->loc, move(method)); - } else if (send->fun == core::Names::describe() && send->numPosArgs() == 1 && send->hasBlock() && - send->block()->args.size() == 0) { - return prepareTestEachBody(ctx, eachName, std::move(send->block()->body), args, - std::move(destructuringStmts), iteratee); + } else if (send->fun == core::Names::describe() && send->numPosArgs() == 1 && correctBlockArity) { + return prepareTestEachBody(ctx, eachName, std::move(send->block()->body), args, destructuringStmts, + iteratee, + /* insideDescribe */ true); + } else if (insideDescribe && send->fun == core::Names::let() && send->numPosArgs() == 1 && correctBlockArity && + ast::isa_tree(send->getPosArg(0))) { + auto argLiteral = ast::cast_tree_nonnull(send->getPosArg(0)); + if (argLiteral.isName()) { + auto declLoc = send->loc.copyWithZeroLength().join(argLiteral.loc); + auto methodName = argLiteral.asName(); + return ast::MK::SyntheticMethod0(send->loc, declLoc, argLiteral.loc, methodName, + std::move(send->block()->body)); + } } } @@ -320,30 +347,32 @@ bool isDestructuringInsSeq(core::GlobalState &gs, const ast::MethodDef::ARGS_sto // this just walks the body of a `test_each` and tries to transform every statement ast::ExpressionPtr prepareTestEachBody(core::MutableContext ctx, core::NameRef eachName, ast::ExpressionPtr body, - ast::MethodDef::ARGS_store &args, ast::InsSeq::STATS_store destructuringStmts, - ast::ExpressionPtr &iteratee) { + const ast::MethodDef::ARGS_store &args, + absl::Span destructuringStmts, + ast::ExpressionPtr &iteratee, bool insideDescribe) { if (auto *bodySeq = ast::cast_tree(body)) { if (isDestructuringInsSeq(ctx, args, bodySeq)) { ENFORCE(destructuringStmts.empty(), "Nested destructuring statements"); - destructuringStmts.reserve(bodySeq->stats.size()); - std::move(bodySeq->stats.begin(), bodySeq->stats.end(), std::back_inserter(destructuringStmts)); - return prepareTestEachBody(ctx, eachName, std::move(bodySeq->expr), args, std::move(destructuringStmts), - iteratee); + return prepareTestEachBody(ctx, eachName, std::move(bodySeq->expr), args, absl::MakeSpan(bodySeq->stats), + iteratee, insideDescribe); } for (auto &exp : bodySeq->stats) { - exp = runUnderEach(ctx, eachName, destructuringStmts, std::move(exp), args, iteratee); + exp = runUnderEach(ctx, eachName, absl::MakeSpan(destructuringStmts), std::move(exp), args, iteratee, + insideDescribe); } - bodySeq->expr = runUnderEach(ctx, eachName, destructuringStmts, std::move(bodySeq->expr), args, iteratee); + bodySeq->expr = runUnderEach(ctx, eachName, absl::MakeSpan(destructuringStmts), std::move(bodySeq->expr), args, + iteratee, insideDescribe); } else { - body = runUnderEach(ctx, eachName, destructuringStmts, std::move(body), args, iteratee); + body = runUnderEach(ctx, eachName, absl::MakeSpan(destructuringStmts), std::move(body), args, iteratee, + insideDescribe); } return body; } -ast::ExpressionPtr runSingle(core::MutableContext ctx, bool isClass, ast::Send *send) { +ast::ExpressionPtr runSingle(core::MutableContext ctx, bool isClass, ast::Send *send, bool insideDescribe) { if (!send->hasBlock()) { return nullptr; } @@ -367,14 +396,12 @@ ast::ExpressionPtr runSingle(core::MutableContext ctx, bool isClass, ast::Send * // can freely copy into methoddef scope auto iteratee = getIteratee(send->getPosArg(0)); // and then reconstruct the send but with a modified body - return ast::MK::Send( - send->loc, ast::MK::Self(send->recv.loc()), send->fun, send->funLoc, 1, - ast::MK::SendArgs( - move(send->getPosArg(0)), - ast::MK::Block(block->loc, - prepareTestEachBody(ctx, send->fun, std::move(block->body), block->args, {}, iteratee), - std::move(block->args))), - send->flags); + auto body = + prepareTestEachBody(ctx, send->fun, std::move(block->body), block->args, {}, iteratee, insideDescribe); + return ast::MK::Send(send->loc, ast::MK::Self(send->recv.loc()), send->fun, send->funLoc, 1, + ast::MK::SendArgs(move(send->getPosArg(0)), + ast::MK::Block(block->loc, std::move(body), std::move(block->args))), + send->flags); } if (send->fun == core::Names::testEachHash() && send->numKwArgs() > 0) { @@ -393,8 +420,8 @@ ast::ExpressionPtr runSingle(core::MutableContext ctx, bool isClass, ast::Send * ConstantMover constantMover; ast::TreeWalk::apply(ctx, constantMover, block->body); auto declLoc = declLocForSendWithBlock(*send); - auto method = addSigVoid( - ast::MK::SyntheticMethod0(send->loc, declLoc, send->funLoc, name, prepareBody(ctx, isClass, std::move(block->body)))); + auto method = addSigVoid(ast::MK::SyntheticMethod0( + send->loc, declLoc, send->funLoc, name, prepareBody(ctx, isClass, std::move(block->body), insideDescribe))); return constantMover.addConstantsToExpression(send->loc, move(method)); } @@ -402,9 +429,9 @@ ast::ExpressionPtr runSingle(core::MutableContext ctx, bool isClass, ast::Send * return nullptr; } auto &arg = send->getPosArg(0); - auto argString = to_s(ctx, arg); if (send->fun == core::Names::describe()) { + auto argString = to_s(ctx, arg); ast::ClassDef::ANCESTORS_store ancestors; // Avoid subclassing the containing context when it's a module, as that will produce an error in typed: false @@ -413,32 +440,44 @@ ast::ExpressionPtr runSingle(core::MutableContext ctx, bool isClass, ast::Send * ancestors.emplace_back(ast::MK::Self(arg.loc())); } - ast::ClassDef::RHS_store rhs; const bool bodyIsClass = true; - rhs.emplace_back(prepareBody(ctx, bodyIsClass, std::move(block->body))); + auto rhs = prepareBody(ctx, bodyIsClass, std::move(block->body), /* insideDescribe */ true); + auto name = ast::MK::UnresolvedConstant(arg.loc(), ast::MK::EmptyTree(), ctx.state.enterNameConstant("")); auto declLoc = declLocForSendWithBlock(*send); - return ast::MK::Class(send->loc, declLoc, std::move(name), std::move(ancestors), std::move(rhs)); + return ast::MK::Class(send->loc, declLoc, std::move(name), std::move(ancestors), + flattenDescribeBody(move(rhs))); } else if (send->fun == core::Names::it()) { + auto argString = to_s(ctx, arg); ConstantMover constantMover; ast::TreeWalk::apply(ctx, constantMover, block->body); auto name = ctx.state.enterNameUTF8(""); const bool bodyIsClass = false; auto declLoc = declLocForSendWithBlock(*send); - auto method = addSigVoid(ast::MK::SyntheticMethod0(send->loc, declLoc, arg.loc(), std::move(name), - prepareBody(ctx, bodyIsClass, std::move(block->body)))); + auto method = addSigVoid( + ast::MK::SyntheticMethod0(send->loc, declLoc, arg.loc(), std::move(name), + prepareBody(ctx, bodyIsClass, std::move(block->body), insideDescribe))); method = ast::MK::InsSeq1(send->loc, send->getPosArg(0).deepCopy(), move(method)); return constantMover.addConstantsToExpression(send->loc, move(method)); + } else if (insideDescribe && send->fun == core::Names::let() && ast::isa_tree(arg)) { + auto argLiteral = ast::cast_tree_nonnull(arg); + if (!argLiteral.isName()) { + return nullptr; + } + + auto declLoc = send->loc.copyWithZeroLength().join(argLiteral.loc); + auto methodName = argLiteral.asName(); + return ast::MK::SyntheticMethod0(send->loc, declLoc, arg.loc(), methodName, std::move(block->body)); } return nullptr; } -ast::ExpressionPtr recurse(core::MutableContext ctx, bool isClass, ast::ExpressionPtr body) { +ast::ExpressionPtr recurse(core::MutableContext ctx, bool isClass, ast::ExpressionPtr body, bool insideDescribe) { auto bodySend = ast::cast_tree(body); if (bodySend) { - auto change = runSingle(ctx, isClass, bodySend); + auto change = runSingle(ctx, isClass, bodySend, insideDescribe); if (change) { return change; } @@ -452,7 +491,8 @@ vector Minitest::run(core::MutableContext ctx, bool isClass, return stats; } - auto exp = runSingle(ctx, isClass, send); + auto insideDescribe = false; + auto exp = runSingle(ctx, isClass, send, insideDescribe); if (exp != nullptr) { stats.emplace_back(std::move(exp)); } diff --git a/test/scip/testdata/minitest.rb b/test/scip/testdata/minitest_1.rb similarity index 100% rename from test/scip/testdata/minitest.rb rename to test/scip/testdata/minitest_1.rb diff --git a/test/scip/testdata/minitest.snapshot.rb b/test/scip/testdata/minitest_1.snapshot.rb similarity index 100% rename from test/scip/testdata/minitest.snapshot.rb rename to test/scip/testdata/minitest_1.snapshot.rb diff --git a/test/scip/testdata/minitest_2.rb b/test/scip/testdata/minitest_2.rb new file mode 100644 index 000000000..91aaf0510 --- /dev/null +++ b/test/scip/testdata/minitest_2.rb @@ -0,0 +1,20 @@ +# typed: true + +class Test + extend T::Sig + def self.test_each(arg, &blk); end + def self.it(name, &blk); end + def self.describe(name, &blk); end +end + +class Foo < Test + # The unclosed `do` block here should be a recoverable parse error. + test_each([[1, 2], [3,4]]) do |(a,b)| + # ^^ error: Hint: this "do" token + + it "it block 1" do + end + + it "it block 2" do + end +end # error: unexpected token "end of file" diff --git a/test/scip/testdata/minitest_2.snapshot.rb b/test/scip/testdata/minitest_2.snapshot.rb new file mode 100644 index 000000000..6040bb350 --- /dev/null +++ b/test/scip/testdata/minitest_2.snapshot.rb @@ -0,0 +1,30 @@ + # typed: true + + class Test +# ^^^^ definition [..] Test# + extend T::Sig +# ^^^^^^ reference [..] Kernel#extend(). + def self.test_each(arg, &blk); end +# ^^^^^^^^^ definition [..] ``#test_each(). + def self.it(name, &blk); end +# ^^ definition [..] ``#it(). + def self.describe(name, &blk); end +# ^^^^^^^^ definition [..] ``#describe(). + end + + class Foo < Test +# ^^^ definition [..] Foo# +# ^^^^ reference [..] Test# + # The unclosed `do` block here should be a recoverable parse error. + test_each([[1, 2], [3,4]]) do |(a,b)| +# ^^^^^^^^^ reference [..] ``#test_each(). + # ^^ error: Hint: this "do" token + + it "it block 1" do +# ^^^^^^^^^^^^ definition [..] Foo#``(). + end + + it "it block 2" do +# ^^^^^^^^^^^^ definition [..] Foo#``(). + end + end # error: unexpected token "end of file" diff --git a/test/scip/testdata/minitest_3.rb b/test/scip/testdata/minitest_3.rb new file mode 100644 index 000000000..cd009cd97 --- /dev/null +++ b/test/scip/testdata/minitest_3.rb @@ -0,0 +1,24 @@ +# typed: true +extend T::Sig + +class Test + extend T::Sig + + def self.test_each(iter, &blk); end + def self.it(name, &blk); end + def self.describe(name, &blk); end + + test_each([[1,2], [3,4]]) do |(a,b)| + + describe "d" do + it "b" do + T.reveal_type(a) # error: Revealed type: `Integer` + end + end + + it "a" do + T.reveal_type(a) # error: Revealed type: `Integer` + end + + end +end diff --git a/test/scip/testdata/minitest_3.snapshot.rb b/test/scip/testdata/minitest_3.snapshot.rb new file mode 100644 index 000000000..f50a81a60 --- /dev/null +++ b/test/scip/testdata/minitest_3.snapshot.rb @@ -0,0 +1,45 @@ + # typed: true + extend T::Sig +#^^^^^^ reference [..] Kernel#extend(). +# ^ reference [..] T# +# ^^^ reference [..] T#Sig# + + class Test +# ^^^^ definition [..] Test# + extend T::Sig +# ^^^^^^ reference [..] Kernel#extend(). + + def self.test_each(iter, &blk); end +# ^^^^^^^^^ definition [..] ``#test_each(). + def self.it(name, &blk); end +# ^^ definition [..] ``#it(). + def self.describe(name, &blk); end +# ^^^^^^^^ definition [..] ``#describe(). + + test_each([[1,2], [3,4]]) do |(a,b)| +# ^^^^^^^^^ reference [..] ``#test_each(). +# ^ definition local 1~#2288740619 +# ^ definition local 1~#416088458 +# ^ definition local 2~#2288740619 +# ^ definition local 2~#416088458 + + describe "d" do + it "b" do +# ^^^ definition [..] Test#``(). + T.reveal_type(a) # error: Revealed type: `Integer` +# ^ reference [..] T# +# ^^^^^^^^^^^ reference [..] ``#reveal_type(). +# ^ reference local 1~#416088458 + end + end + + it "a" do +# ^^^ definition [..] Test#``(). + T.reveal_type(a) # error: Revealed type: `Integer` +# ^ reference [..] T# +# ^^^^^^^^^^^ reference [..] ``#reveal_type(). +# ^ reference local 1~#2288740619 + end + + end + end