From 192d3734295503d87a764db8adbdce0545e84b60 Mon Sep 17 00:00:00 2001 From: Peter Leonov Date: Mon, 24 Apr 2023 20:49:43 +0000 Subject: [PATCH 1/4] first test --- .../resolver/delegates_missing_methods_to.rb | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 test/testdata/resolver/delegates_missing_methods_to.rb diff --git a/test/testdata/resolver/delegates_missing_methods_to.rb b/test/testdata/resolver/delegates_missing_methods_to.rb new file mode 100644 index 00000000000..e0dde2ded1c --- /dev/null +++ b/test/testdata/resolver/delegates_missing_methods_to.rb @@ -0,0 +1,28 @@ +# typed: true + +class Foo + extend T::Sig + extend T::Helpers + + def foo + 42 + end + + def method_missing(name) + Bar.new.send(name) + end + + delegates_missing_methods_to { Bar } +end + +class Bar + def bar + 12 + end +end + +foo = Foo.new +# no Sorbet error +foo.foo #=> 42 +# no Sorbet error +foo.bar #=> 12 From 401eac860a20838c3cca37619814a2248a5ef5b7 Mon Sep 17 00:00:00 2001 From: Peter Leonov Date: Mon, 24 Apr 2023 20:50:07 +0000 Subject: [PATCH 2/4] copy requires_ancestor --- core/tools/generate_names.cc | 3 + resolver/resolver.cc | 130 +++++++++++++++++++++++++++++++++++ 2 files changed, 133 insertions(+) diff --git a/core/tools/generate_names.cc b/core/tools/generate_names.cc index 82a6dee05a7..cce9f87cc7a 100644 --- a/core/tools/generate_names.cc +++ b/core/tools/generate_names.cc @@ -328,6 +328,9 @@ NameDef names[] = { {"requiredAncestorsLin", ""}, {"requiresAncestor", "requires_ancestor"}, + // Delegate methods + {"delegatesMissingMethodsTo", "delegates_missing_methods_to"}, + // This behaves like the above two names, in the sense that we use a member // on a class to lookup an associated symbol with some extra info. {"sealedSubclasses", "sealed_subclasses"}, diff --git a/resolver/resolver.cc b/resolver/resolver.cc index 9b6f4e2f97f..b663f542818 100644 --- a/resolver/resolver.cc +++ b/resolver/resolver.cc @@ -237,12 +237,30 @@ class ResolveConstantsWalk { const RequireAncestorResolutionItem &operator=(const RequireAncestorResolutionItem &) = delete; }; + struct DelegatesMissingMethodsToResolutionItem { + core::FileRef file; + core::ClassOrModuleRef owner; + ast::Send *send; + + DelegatesMissingMethodsToResolutionItem(core::FileRef file, core::ClassOrModuleRef owner, ast::Send *send) + : file(file), owner(owner), send(send) {} + + DelegatesMissingMethodsToResolutionItem(DelegatesMissingMethodsToResolutionItem &&) noexcept = default; + DelegatesMissingMethodsToResolutionItem & + operator=(DelegatesMissingMethodsToResolutionItem &&rhs) noexcept = default; + + DelegatesMissingMethodsToResolutionItem(const DelegatesMissingMethodsToResolutionItem &) = delete; + const DelegatesMissingMethodsToResolutionItem & + operator=(const DelegatesMissingMethodsToResolutionItem &) = delete; + }; + vector todo_; vector todoAncestors_; vector todoClassAliases_; vector todoTypeAliases_; vector todoClassMethods_; vector todoRequiredAncestors_; + vector missingMethodsDelegatees_; static core::SymbolRef resolveLhs(core::Context ctx, const shared_ptr &nesting, core::NameRef name) { Nesting *scope = nesting.get(); @@ -1285,6 +1303,102 @@ class ResolveConstantsWalk { owner.data(gs)->recordRequiredAncestor(gs, symbol, blockLoc); } + static void resolveMissingMethodsDelegateesJob(core::GlobalState &gs, + const DelegatesMissingMethodsToResolutionItem &todo) { + auto owner = todo.owner; + auto send = todo.send; + auto loc = core::Loc(todo.file, send->loc); + + if (!owner.data(gs)->isModule() && !owner.data(gs)->flags.isAbstract) { + if (auto e = gs.beginError(loc, core::errors::Resolver::InvalidRequiredAncestor)) { + e.setHeader("`{}` can only be declared inside a module or an abstract class", send->fun.show(gs)); + } + return; + } + + auto *block = send->block(); + + if (send->numPosArgs() > 0) { + if (auto e = gs.beginError(loc, core::errors::Resolver::InvalidRequiredAncestor)) { + e.setHeader("`{}` only accepts a block", send->fun.show(gs)); + e.addErrorNote("Use {} to auto-correct using the new syntax", + "--isolate-error-code 5062 -a --typed true"); + + if (block != nullptr) { + return; + } + + string replacement = ""; + int indent = core::Loc::offset2Pos(todo.file.data(gs), send->loc.beginPos()).column - 1; + int index = 1; + const auto numPosArgs = send->numPosArgs(); + for (auto i = 0; i < numPosArgs; ++i) { + auto &arg = send->getPosArg(i); + auto argLoc = core::Loc(todo.file, arg.loc()); + replacement += fmt::format("{:{}}{} {{ {} }}{}", "", index == 1 ? 0 : indent, send->fun.show(gs), + argLoc.source(gs).value(), index < numPosArgs ? "\n" : ""); + index += 1; + } + e.addAutocorrect( + core::AutocorrectSuggestion{fmt::format("Replace `{}` with `{}`", send->fun.show(gs), replacement), + {core::AutocorrectSuggestion::Edit{loc, replacement}}}); + } + return; + } + + if (block == nullptr) { + return; // The sig mismatch error will be emitted later by infer. + } + + ENFORCE(block->body); + + auto blockLoc = core::Loc(todo.file, block->body.loc()); + core::ClassOrModuleRef symbol = core::Symbols::StubModule(); + + if (auto *constant = ast::cast_tree(block->body)) { + if (constant->symbol.exists() && constant->symbol.isClassOrModule()) { + symbol = constant->symbol.asClassOrModuleRef(); + } + } else if (isTClassOf(block->body)) { + send = ast::cast_tree(block->body); + + ENFORCE(send); + + if (send->numPosArgs() == 1) { + if (auto *argClass = ast::cast_tree(send->getPosArg(0))) { + if (argClass->symbol.exists() && argClass->symbol.isClassOrModule()) { + if (argClass->symbol == owner) { + if (auto e = gs.beginError(blockLoc, core::errors::Resolver::InvalidRequiredAncestor)) { + e.setHeader("Must not pass yourself to `{}` inside of `requires_ancestor`", + send->fun.show(gs)); + } + return; + } + + symbol = argClass->symbol.asClassOrModuleRef().data(gs)->lookupSingletonClass(gs); + } + } + } + } + + if (symbol == core::Symbols::StubModule()) { + if (auto e = gs.beginError(blockLoc, core::errors::Resolver::InvalidRequiredAncestor)) { + e.setHeader("Argument to `{}` must be statically resolvable to a class or a module", + send->fun.show(gs)); + } + return; + } + + if (symbol == owner) { + if (auto e = gs.beginError(blockLoc, core::errors::Resolver::InvalidRequiredAncestor)) { + e.setHeader("Must not pass yourself to `{}`", send->fun.show(gs)); + } + return; + } + + owner.data(gs)->recordRequiredAncestor(gs, symbol, blockLoc); + } + static void tryRegisterSealedSubclass(core::MutableContext ctx, AncestorResolutionItem &job) { ENFORCE(job.ancestor->symbol.exists(), "Ancestor must exist, or we can't check whether it's sealed."); auto ancestorSym = job.ancestor->symbol.dealias(ctx).asClassOrModuleRef(); @@ -1658,6 +1772,11 @@ class ResolveConstantsWalk { if (ctx.state.requiresAncestorEnabled) { this->todoRequiredAncestors_.emplace_back(ctx.file, ctx.owner.asClassOrModuleRef(), &send); } + } else if (send.fun == core::Names::delegatesMissingMethodsTo()) { + // printf("!!!!!!!!!!!!!!!!!!!!!\n"); + // printf("delegatesMissingMethodsTo"); + // printf("!!!!!!!!!!!!!!!!!!!!!\n"); + this->missingMethodsDelegatees_.emplace_back(ctx.file, ctx.owner.asClassOrModuleRef(), &send); } } else { auto recvAsConstantLit = ast::cast_tree(send.recv); @@ -1775,6 +1894,7 @@ class ResolveConstantsWalk { vector> todoTypeAliases; vector> todoClassMethods; vector> todoRequiredAncestors; + vector> todoMissingMethodsDelegatees; { ResolveWalkResult threadResult; @@ -1917,6 +2037,16 @@ class ResolveConstantsWalk { todoRequiredAncestors.clear(); } + { + Timer timeit(gs.tracer(), "resolver.delegates_missing_methods_to"); + for (auto &job : todoMissingMethodsDelegatees) { + for (auto &todo : job.items) { + resolveMissingMethodsDelegateesJob(gs, todo); + } + } + todoRequiredAncestors.clear(); + } + // We can no longer resolve new constants. All the code below reports errors categoryCounterAdd("resolve.constants.nonancestor", "failure", todo.size()); From 5296e758e937d7f4048be67af6ee5bdcbba2cd58 Mon Sep 17 00:00:00 2001 From: Peter Leonov Date: Fri, 28 Apr 2023 09:59:52 +0000 Subject: [PATCH 3/4] test include too --- .../resolver/delegates_missing_methods_to.rb | 34 ++++++++++++------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/test/testdata/resolver/delegates_missing_methods_to.rb b/test/testdata/resolver/delegates_missing_methods_to.rb index e0dde2ded1c..d8338ae9adf 100644 --- a/test/testdata/resolver/delegates_missing_methods_to.rb +++ b/test/testdata/resolver/delegates_missing_methods_to.rb @@ -1,28 +1,36 @@ # typed: true -class Foo +class Delegatee + def delegated_method; end +end + +class Direct extend T::Sig extend T::Helpers - def foo - 42 + def method_missing(name) + Delegatee.new.send(name) end + delegates_missing_methods_to { Bar } +end + +Direct.new.delegated_method + + +class DelegatingModule + extend T::Sig + extend T::Helpers + def method_missing(name) - Bar.new.send(name) + Delegatee.new.send(name) end delegates_missing_methods_to { Bar } end -class Bar - def bar - 12 - end +class ViaModule + include DelegatingModule end -foo = Foo.new -# no Sorbet error -foo.foo #=> 42 -# no Sorbet error -foo.bar #=> 12 +ViaModule.new.delegated_method From 72694851deec4664d523e15ae03735395cf8e44e Mon Sep 17 00:00:00 2001 From: Peter Leonov Date: Fri, 28 Apr 2023 10:00:28 +0000 Subject: [PATCH 4/4] WIP --- core/errors/resolver.h | 1 + resolver/resolver.cc | 23 ++++++++++++----------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/core/errors/resolver.h b/core/errors/resolver.h index a53793f7abc..eafefd9237a 100644 --- a/core/errors/resolver.h +++ b/core/errors/resolver.h @@ -80,6 +80,7 @@ constexpr ErrorClass BindNonBlockParameter{5071, StrictLevel::False}; constexpr ErrorClass TypeMemberScopeMismatch{5072, StrictLevel::False}; constexpr ErrorClass AbstractClassInstantiated{5073, StrictLevel::True}; constexpr ErrorClass HasAttachedClassIncluded{5074, StrictLevel::False}; +constexpr ErrorClass InvalidDelegatesMissingMethodsTo{5075, StrictLevel::True}; } // namespace sorbet::core::errors::Resolver #endif diff --git a/resolver/resolver.cc b/resolver/resolver.cc index b663f542818..ea6bbe40fb4 100644 --- a/resolver/resolver.cc +++ b/resolver/resolver.cc @@ -1309,9 +1309,9 @@ class ResolveConstantsWalk { auto send = todo.send; auto loc = core::Loc(todo.file, send->loc); - if (!owner.data(gs)->isModule() && !owner.data(gs)->flags.isAbstract) { - if (auto e = gs.beginError(loc, core::errors::Resolver::InvalidRequiredAncestor)) { - e.setHeader("`{}` can only be declared inside a module or an abstract class", send->fun.show(gs)); + if (owner.data(gs)->flags.isAbstract) { + if (auto e = gs.beginError(loc, core::errors::Resolver::InvalidDelegatesMissingMethodsTo)) { + e.setHeader("`{}` can not be declared inside an abstract class", send->fun.show(gs)); } return; } @@ -1319,10 +1319,10 @@ class ResolveConstantsWalk { auto *block = send->block(); if (send->numPosArgs() > 0) { - if (auto e = gs.beginError(loc, core::errors::Resolver::InvalidRequiredAncestor)) { + if (auto e = gs.beginError(loc, core::errors::Resolver::InvalidDelegatesMissingMethodsTo)) { e.setHeader("`{}` only accepts a block", send->fun.show(gs)); e.addErrorNote("Use {} to auto-correct using the new syntax", - "--isolate-error-code 5062 -a --typed true"); + "--isolate-error-code 5075 -a --typed true"); if (block != nullptr) { return; @@ -1368,8 +1368,9 @@ class ResolveConstantsWalk { if (auto *argClass = ast::cast_tree(send->getPosArg(0))) { if (argClass->symbol.exists() && argClass->symbol.isClassOrModule()) { if (argClass->symbol == owner) { - if (auto e = gs.beginError(blockLoc, core::errors::Resolver::InvalidRequiredAncestor)) { - e.setHeader("Must not pass yourself to `{}` inside of `requires_ancestor`", + if (auto e = + gs.beginError(blockLoc, core::errors::Resolver::InvalidDelegatesMissingMethodsTo)) { + e.setHeader("Must not pass yourself to `{}` inside of `delegates_missing_methods_to`", send->fun.show(gs)); } return; @@ -1382,20 +1383,20 @@ class ResolveConstantsWalk { } if (symbol == core::Symbols::StubModule()) { - if (auto e = gs.beginError(blockLoc, core::errors::Resolver::InvalidRequiredAncestor)) { - e.setHeader("Argument to `{}` must be statically resolvable to a class or a module", - send->fun.show(gs)); + if (auto e = gs.beginError(blockLoc, core::errors::Resolver::InvalidDelegatesMissingMethodsTo)) { + e.setHeader("Argument to `{}` must be statically resolvable to a class", send->fun.show(gs)); } return; } if (symbol == owner) { - if (auto e = gs.beginError(blockLoc, core::errors::Resolver::InvalidRequiredAncestor)) { + if (auto e = gs.beginError(blockLoc, core::errors::Resolver::InvalidDelegatesMissingMethodsTo)) { e.setHeader("Must not pass yourself to `{}`", send->fun.show(gs)); } return; } + // STOPPED COPYPASTING HERE owner.data(gs)->recordRequiredAncestor(gs, symbol, blockLoc); }