From d2fedccc327c1b59fbf8195520a383ac2dd6b587 Mon Sep 17 00:00:00 2001 From: Varun Gandhi Date: Fri, 16 Jun 2023 14:06:17 +0800 Subject: [PATCH] fix: Remove incorrect diagnostic for unknown field --- core/GlobalState.h | 2 ++ resolver/resolver.cc | 9 +++++++++ scip_indexer/SCIPFieldResolve.cc | 18 ------------------ 3 files changed, 11 insertions(+), 18 deletions(-) diff --git a/core/GlobalState.h b/core/GlobalState.h index 61f15b1db..f75d4c042 100644 --- a/core/GlobalState.h +++ b/core/GlobalState.h @@ -303,6 +303,8 @@ class GlobalState final { bool isSCIPRuby = true; // --- begin scip-ruby specific state + // Mapping describing which field references in a class are still + // marked as "unresolved" after name resolution is complete. UnorderedMap> unresolvedFields; // --- end scip-ruby specific state diff --git a/resolver/resolver.cc b/resolver/resolver.cc index ca7962d94..097ed109a 100644 --- a/resolver/resolver.cc +++ b/resolver/resolver.cc @@ -4188,6 +4188,15 @@ class CollectUnresolvedFieldsWalk final { gs.unresolvedFields.insert(make_move_iterator(threadResult.unresolvedFields.begin()), make_move_iterator(threadResult.unresolvedFields.end())); } + // NOTE(varun): This walker is meant to be invoked after name resolution is finished. + // As such, one might expect that the unresolved fields across classes stay the same + // across runs, as all the single-threaded merging which requires cross-file data + // is complete. However, that's not the case. 😕 + // + // In particular, when testing on the shopify-ruby-codebase, printing the + // printing the number of unresolved fields for each class gave varying results + // (e.g. in one run, a class would have 85 unresolved fields, in another run, + // it would have 87 unresolved fields). } fast_sort(trees, [](const auto &lhs, const auto &rhs) -> bool { return lhs.file < rhs.file; }); diff --git a/scip_indexer/SCIPFieldResolve.cc b/scip_indexer/SCIPFieldResolve.cc index f76d24351..a3e5f7c56 100644 --- a/scip_indexer/SCIPFieldResolve.cc +++ b/scip_indexer/SCIPFieldResolve.cc @@ -78,24 +78,6 @@ core::ClassOrModuleRef FieldResolver::findUnresolvedFieldInInheritanceChain(cons } start = FieldResolver::normalizeParentForClassVar(gs, start, fieldText); - if (gs.unresolvedFields.find(start) == gs.unresolvedFields.end() || - !gs.unresolvedFields.find(start)->second.contains(field)) { - // Triggered by code patterns like: - // # top-level - // def MyClass.method - // # blah - // end - // which is not supported by Sorbet. - LOG_DEBUG(gs, debugLoc, - fmt::format("couldn't find field {} in class {};\n" - "are you using a code pattern like def MyClass.method which is unsupported by Sorbet?", - field.exists() ? field.toString(gs) : "", - start.exists() ? start.showFullName(gs) : "")); - // As a best-effort guess, assume that the definition is - // in this class but we somehow missed it. - return start; - } - auto best = start; auto cur = start; while (cur.exists()) {