Merge pull request #184 from github/test-new-partial-paths

Change tests and C API to use minimal partial paths set

Author: Hendrik van Antwerpen

…st/builtins/async-arrow-function.ts.skip …pt/test/builtins/async-arrow-function.tslanguages/tree-sitter-stack-graphs-typescript/test/builtins/async-arrow-function.ts.skip renamed to languages/tree-sitter-stack-graphs-typescript/test/builtins/async-arrow-function.ts languages/tree-sitter-stack-graphs-typescript/test/builtins/async-arrow-function.ts.skip renamed to languages/tree-sitter-stack-graphs-typescript/test/builtins/async-arrow-function.ts

@@ -1576,45 +1576,17 @@ pub extern "C" fn sg_partial_path_arena_find_partial_paths_in_file(
     let partial_path_list = unsafe { &mut *partial_path_list };
     let cancellation_flag: Option<&AtomicUsize> =
         unsafe { std::mem::transmute(cancellation_flag.as_ref()) };
-    sg_partial_path_arena_find_partial_paths_in_file_inner(
-        graph,
-        partials,
-        file,
-        partial_path_list,
-        &AtomicUsizeCancellationFlag(cancellation_flag),
-    )
-    .into()
-}
-
-fn sg_partial_path_arena_find_partial_paths_in_file_inner(
-    graph: &StackGraph,
-    partials: &mut PartialPaths,
-    file: Handle<File>,
-    partial_path_list: &mut sg_partial_path_list,
-    cancellation_flag: &dyn CancellationFlag,
-) -> Result<(), CancellationError> {
-    let mut db = Database::new();
-    partials.find_minimal_partial_paths_set_in_file(
-        graph,
-        file,
-        cancellation_flag,
-        |graph, partials, path| {
-            db.add_partial_path(graph, partials, path);
-        },
-    )?;
-    #[allow(deprecated)]
-    ForwardPartialPathStitcher::find_locally_complete_partial_paths(
-        graph,
-        partials,
-        &mut db,
-        cancellation_flag,
-        |_graph, partials, path| {
-            let mut path = path.clone();
-            path.ensure_both_directions(partials);
-            partial_path_list.partial_paths.push(path);
-        },
-    )?;
-    Ok(())
+    partials
+        .find_minimal_partial_paths_set_in_file(
+            graph,
+            file,
+            &AtomicUsizeCancellationFlag(cancellation_flag),
+            |_graph, partials, mut path| {
+                path.ensure_both_directions(partials);
+                partial_path_list.partial_paths.push(path);
+            },
+        )
+        .into()
 }
 
 /// A handle to a partial path in a partial path database.  A zero handle represents a missing

…iltins/async-function-definition.ts.skip …st/builtins/async-function-definition.tslanguages/tree-sitter-stack-graphs-typescript/test/builtins/async-function-definition.ts.skip renamed to languages/tree-sitter-stack-graphs-typescript/test/builtins/async-function-definition.ts languages/tree-sitter-stack-graphs-typescript/test/builtins/async-function-definition.ts.skip renamed to languages/tree-sitter-stack-graphs-typescript/test/builtins/async-function-definition.ts

@@ -725,11 +725,9 @@ pub struct ForwardPartialPathStitcher {
     max_work_per_phase: usize,
     #[cfg(feature = "copious-debugging")]
     phase_number: usize,
-    should_extend: Box<ShouldExtend>,
+    should_extend: fn(&StackGraph, &mut PartialPaths, &PartialPath) -> bool,
 }
 
-type ShouldExtend = dyn Fn(&StackGraph, &mut PartialPaths, &PartialPath) -> bool;
-
 impl ForwardPartialPathStitcher {
     /// Creates a new forward partial path stitcher that is "seeded" with a set of starting stack
     /// graph nodes.
@@ -787,7 +785,7 @@ impl ForwardPartialPathStitcher {
             max_work_per_phase: usize::MAX,
             #[cfg(feature = "copious-debugging")]
             phase_number: 1,
-            should_extend: Box::new(|_, _, _| true),
+            should_extend: |_, _, _| true,
         }
     }
 
@@ -813,7 +811,7 @@ impl ForwardPartialPathStitcher {
             max_work_per_phase: usize::MAX,
             #[cfg(feature = "copious-debugging")]
             phase_number: 1,
-            should_extend: Box::new(|_, _, _| true),
+            should_extend: |_, _, _| true,
         }
     }
 
@@ -846,6 +844,14 @@ impl ForwardPartialPathStitcher {
         self.max_work_per_phase = max_work_per_phase;
     }
 
+    /// Sets a condition that determines if a partial path is a candidate that should be extended.
+    pub fn set_should_extend(
+        &mut self,
+        should_extend: fn(&StackGraph, &mut PartialPaths, &PartialPath) -> bool,
+    ) {
+        self.should_extend = should_extend;
+    }
+
     /// Attempts to extend one partial path as part of the algorithm.  When calling this function,
     /// you are responsible for ensuring that `db` already contains all of the possible partial
     /// paths that we might want to extend `partial_path` with.
@@ -896,15 +902,13 @@ impl ForwardPartialPathStitcher {
                     copious_debugging!("        is invalid: {:?}", err);
                     continue;
                 }
-                if new_partial_path.start_node != partial_path.start_node {
-                    copious_debugging!("        is invalid: slips off of starting node");
+                if !new_partial_path.is_productive(partials) {
+                    copious_debugging!("        is invalid: not productive");
                     continue;
                 }
             }
             copious_debugging!("        is {}", new_partial_path.display(graph, partials));
-            if new_partial_path.is_productive(partials) {
-                self.next_iteration.push_back(new_partial_path);
-            }
+            self.next_iteration.push_back(new_partial_path);
         }
 
         extension_count
@@ -988,6 +992,7 @@ impl ForwardPartialPathStitcher {
     {
         let mut stitcher =
             ForwardPartialPathStitcher::from_nodes(graph, partials, db, starting_nodes);
+        stitcher.set_should_extend(|g, _, p| p.starts_at_reference(g));
         while !stitcher.is_complete() {
             cancellation_flag.check("finding complete partial paths")?;
             let complete_partial_paths = stitcher
@@ -1000,70 +1005,4 @@ impl ForwardPartialPathStitcher {
         }
         Ok(())
     }
-
-    /// Finds a set of locally complete partial paths, calling the `visit` closure for each one.
-    ///
-    /// This functions computes paths that
-    ///  (a) start at references, exported scopes, or the root,
-    ///  (b) end at definitions, exported scopes, jump-to-scope nodes, or the root, and
-    ///  (c) do not go through the root node.
-    /// If the partial paths in de provided database are file-local, the resutling paths set will
-    /// contain all paths that are locally complete, or end at a file boundary. The set of paths
-    /// may contain paths that can be constructed by stitching other paths in the result set.
-    ///
-    /// This function will not return until all reachable partial paths have been processed, so
-    /// your database must already contain all partial paths that might be needed.  If you have a
-    /// very large stack graph stored in some other storage system, and want more control over
-    /// lazily loading only the necessary pieces, then you should code up your own loop that calls
-    /// [`process_next_phase`][] manually.
-    ///
-    /// [`process_next_phase`]: #method.process_next_phase
-    #[deprecated = "This method replicates old PartialPaths::find_all_partial_paths_in_file behavior. It has poor performance because it computes more paths than necessary, and users should use PartialPaths::find_minimal_partial_paths_set_in_file instead."]
-    pub fn find_locally_complete_partial_paths<F>(
-        graph: &StackGraph,
-        partials: &mut PartialPaths,
-        db: &mut Database,
-        cancellation_flag: &dyn CancellationFlag,
-        mut visit: F,
-    ) -> Result<(), CancellationError>
-    where
-        F: FnMut(&StackGraph, &mut PartialPaths, &PartialPath),
-    {
-        fn is_start_node(graph: &StackGraph, node: Handle<Node>) -> bool {
-            let node = &graph[node];
-            node.is_reference() || node.is_exported_scope() || node.is_root()
-        }
-        fn is_complete(graph: &StackGraph, path: &PartialPath) -> bool {
-            let start_node = &graph[path.start_node];
-            let end_node = &graph[path.end_node];
-            let start_ok = path.starts_at_reference(graph)
-                || start_node.is_root()
-                || start_node.is_exported_scope();
-            let end_ok = path.ends_at_definition(graph)
-                || end_node.is_exported_scope()
-                || end_node.is_root()
-                || end_node.is_jump_to();
-            start_ok && end_ok
-        }
-        let starting_nodes = graph
-            .iter_nodes()
-            .filter(|node| is_start_node(graph, *node))
-            .collect::<Vec<_>>();
-        let mut stitcher =
-            ForwardPartialPathStitcher::from_nodes(graph, partials, db, starting_nodes);
-        stitcher.should_extend =
-            Box::new(|g, _ps, p| p.edges.is_empty() || !g[p.end_node].is_root());
-        while !stitcher.is_complete() {
-            cancellation_flag.check("finding complete partial paths")?;
-            let partial_paths = stitcher
-                .previous_phase_partial_paths()
-                .filter(|partial_path| is_complete(graph, partial_path))
-                .collect::<Vec<_>>();
-            for path in partial_paths {
-                visit(graph, partials, path);
-            }
-            stitcher.process_next_phase(graph, partials, db);
-        }
-        Ok(())
-    }
 }

…iltins/async-function-expression.ts.skip …st/builtins/async-function-expression.tslanguages/tree-sitter-stack-graphs-typescript/test/builtins/async-function-expression.ts.skip renamed to languages/tree-sitter-stack-graphs-typescript/test/builtins/async-function-expression.ts languages/tree-sitter-stack-graphs-typescript/test/builtins/async-function-expression.ts.skip renamed to languages/tree-sitter-stack-graphs-typescript/test/builtins/async-function-expression.ts

@@ -142,64 +142,17 @@ fn class_field_through_function_parameter() {
     check_partial_paths_in_file(
         &graph,
         "main.py",
-        &[
-            // definition of `__main__` module
-            "<__main__,%1> ($1) [root] -> [main.py(0) definition __main__] <%1> ($1)",
-            // reference to `a` in import statement
-            "<%1> ($1) [main.py(17) reference a] -> [root] <a,%1> ($1)",
-            // `from a import *` means we can rewrite any lookup of `__main__.*` → `a.*`
-            "<__main__.,%2> ($1) [root] -> [root] <a.,%2> ($1)",
-            // reference to `b` in import statement
-            "<%1> ($1) [main.py(15) reference b] -> [root] <b,%1> ($1)",
-            // `from b import *` means we can rewrite any lookup of `__main__.*` → `b.*`
-            "<__main__.,%2> ($1) [root] -> [root] <b.,%2> ($1)",
-            // we can look for every reference in either `a` or `b`
-            "<%1> ($1) [main.py(9) reference A] -> [root] <a.A,%1> ($1)",
-            "<%1> ($1) [main.py(9) reference A] -> [root] <b.A,%1> ($1)",
-            "<%1> ($1) [main.py(10) reference bar] -> [root] <a.foo()/([main.py(7)],$1).bar,%1> ($1)",
-            "<%1> ($1) [main.py(10) reference bar] -> [root] <b.foo()/([main.py(7)],$1).bar,%1> ($1)",
-            "<%1> ($1) [main.py(13) reference foo] -> [root] <a.foo,%1> ($1)",
-            "<%1> ($1) [main.py(13) reference foo] -> [root] <b.foo,%1> ($1)",
-            // parameter 0 of function call is `A`, which we can look up in either `a` or `b`
-            "<0,%1> ($1) [main.py(7) exported scope] -> [root] <a.A,%1> ($1)",
-            "<0,%1> ($1) [main.py(7) exported scope] -> [root] <b.A,%1> ($1)",
-        ],
+        crate::can_find_partial_paths_in_file::CLASS_FIELD_THROUGH_FUNCTION_PARAMETER_MAIN_PATHS,
     );
     check_partial_paths_in_file(
         &graph,
         "a.py",
-        &[
-            // definition of `a` module
-            "<a,%1> ($1) [root] -> [a.py(0) definition a] <%1> ($1)",
-            // definition of `foo` function
-            "<a.foo,%2> ($1) [root] -> [a.py(5) definition foo] <%2> ($1)",
-            // reference to `x` in function body can resolve to formal parameter, and may have passed in parameters...
-            "<%1> ($1) [a.py(8) reference x] -> [a.py(14) definition x] <%1> ()",
-            // ...which we can look up either the 0th actual positional parameter...
-            "<%1> ($1) [a.py(8) reference x] -> [jump to scope] <0,%1> ($1)",
-            // ...or the actual named parameter `x`
-            "<%1> ($1) [a.py(8) reference x] -> [jump to scope] <x,%1> ($1)",
-            // result of function is `x`, which is passed in as a formal parameter...
-            "<a.foo()/($3),%3> ($1) [root] -> [a.py(14) definition x] <%3> ()",
-            // ...which we can look up either the 0th actual positional parameter...
-            "<a.foo()/($3),%3> ($1) [root] -> [jump to scope] <0,%3> ($3)",
-            // ...or the actual named parameter `x`
-            "<a.foo()/($3),%3> ($1) [root] -> [jump to scope] <x,%3> ($3)",
-        ],
+        crate::can_find_partial_paths_in_file::CLASS_FIELD_THROUGH_FUNCTION_PARAMETER_A_PATHS,
     );
     check_partial_paths_in_file(
         &graph,
         "b.py",
-        &[
-            // definition of `b` module
-            "<b,%1> ($1) [root] -> [b.py(0) definition b] <%1> ($1)",
-            // definition of class `A`
-            "<b.A,%2> ($1) [root] -> [b.py(5) definition A] <%2> ($1)",
-            // definition of class member `A.bar`
-            "<b.A.bar,%3> ($1) [root] -> [b.py(8) definition bar] <%3> ($1)",
-            // `bar` can also be accessed as an instance member
-            "<b.A()/($3).bar,%3> ($1) [root] -> [b.py(8) definition bar] <%3> ($3)",
-        ],
+        crate::can_find_partial_paths_in_file::CLASS_FIELD_THROUGH_FUNCTION_PARAMETER_B_PATHS,
     );
 }
 
@@ -209,42 +162,17 @@ fn cyclic_imports_python() {
     check_partial_paths_in_file(
         &graph,
         "main.py",
-        &[
-            // definition of `__main__` module
-            "<__main__,%1> ($1) [root] -> [main.py(0) definition __main__] <%1> ($1)",
-            // reference to `a` in import statement
-            "<%1> ($1) [main.py(8) reference a] -> [root] <a,%1> ($1)",
-            // `from a import *` means we can rewrite any lookup of `__main__.*` → `a.*`
-            "<__main__.,%2> ($1) [root] -> [root] <a.,%2> ($1)",
-            // reference to `foo` becomes `a.foo` because of import statement
-            "<%1> ($1) [main.py(6) reference foo] -> [root] <a.foo,%1> ($1)",
-        ],
+        crate::can_find_partial_paths_in_file::CYCLIC_IMPORTS_PYTHON_MAIN_PATHS,
     );
     check_partial_paths_in_file(
         &graph,
         "a.py",
-        &[
-            // definition of `a` module
-            "<a,%1> ($1) [root] -> [a.py(0) definition a] <%1> ($1)",
-            // reference to `b` in import statement
-            "<%1> ($1) [a.py(6) reference b] -> [root] <b,%1> ($1)",
-            // `from b import *` means we can rewrite any lookup of `a.*` → `b.*`
-            "<a.,%2> ($1) [root] -> [root] <b.,%2> ($1)",
-        ],
+        crate::can_find_partial_paths_in_file::CYCLIC_IMPORTS_PYTHON_A_PATHS,
     );
     check_partial_paths_in_file(
         &graph,
         "b.py",
-        &[
-            // definition of `b` module
-            "<b,%1> ($1) [root] -> [b.py(0) definition b] <%1> ($1)",
-            // reference to `a` in import statement
-            "<%1> ($1) [b.py(8) reference a] -> [root] <a,%1> ($1)",
-            // `from a import *` means we can rewrite any lookup of `b.*` → `a.*`
-            "<b.,%2> ($1) [root] -> [root] <a.,%2> ($1)",
-            // definition of `foo`
-            "<b.foo,%2> ($1) [root] -> [b.py(6) definition foo] <%2> ($1)",
-        ],
+        crate::can_find_partial_paths_in_file::CYCLIC_IMPORTS_PYTHON_B_PATHS,
     );
 }
 
@@ -254,21 +182,7 @@ fn cyclic_imports_rust() {
     check_partial_paths_in_file(
         &graph,
         "test.rs",
-        // NOTE: Because everything in this example is local to one file, there aren't any partial
-        // paths involving the root node.
-        &[
-            // reference to `a` in `main` function
-            "<%1> ($1) [test.rs(103) reference a] -> [test.rs(201) definition a] <%1> ($1)",
-            // reference to `a` in `b` function
-            "<%1> ($1) [test.rs(307) reference a] -> [test.rs(201) definition a] <%1> ($1)",
-            // reference to `b` in `a` function
-            "<%1> ($1) [test.rs(206) reference b] -> [test.rs(301) definition b] <%1> ($1)",
-            // reference to `FOO` in `main` can resolve either to `a::BAR` or `b::FOO`
-            "<%1> ($1) [test.rs(101) reference FOO] -> [test.rs(204) definition BAR] <%1> ($1)",
-            "<%1> ($1) [test.rs(101) reference FOO] -> [test.rs(304) definition FOO] <%1> ($1)",
-            // reference to `BAR` in `b` resolves _only_ to `a::BAR`
-            "<%1> ($1) [test.rs(305) reference BAR] -> [test.rs(204) definition BAR] <%1> ($1)",
-        ],
+        crate::can_find_partial_paths_in_file::CYCLIC_IMPORTS_RUST_PATHS,
     );
 }
 
@@ -278,37 +192,16 @@ fn sequenced_import_star() {
     check_partial_paths_in_file(
         &graph,
         "main.py",
-        &[
-            // definition of `__main__` module
-            "<__main__,%1> ($1) [root] -> [main.py(0) definition __main__] <%1> ($1)",
-            // reference to `a` in import statement
-            "<%1> ($1) [main.py(8) reference a] -> [root] <a,%1> ($1)",
-            // `from a import *` means we can rewrite any lookup of `__main__.*` → `a.*`
-            "<__main__.,%2> ($1) [root] -> [root] <a.,%2> ($1)",
-            // reference to `foo` becomes `a.foo` because of import statement
-            "<%1> ($1) [main.py(6) reference foo] -> [root] <a.foo,%1> ($1)",
-        ],
+        crate::can_find_partial_paths_in_file::SEQUENCED_IMPORT_STAR_MAIN_PATHS,
     );
     check_partial_paths_in_file(
         &graph,
         "a.py",
-        &[
-            // definition of `a` module
-            "<a,%1> ($1) [root] -> [a.py(0) definition a] <%1> ($1)",
-            // reference to `b` in import statement
-            "<%1> ($1) [a.py(6) reference b] -> [root] <b,%1> ($1)",
-            // `from b import *` means we can rewrite any lookup of `a.*` → `b.*`
-            "<a.,%2> ($1) [root] -> [root] <b.,%2> ($1)",
-        ],
+        crate::can_find_partial_paths_in_file::SEQUENCED_IMPORT_STAR_A_PATHS,
     );
     check_partial_paths_in_file(
         &graph,
         "b.py",
-        &[
-            // definition of `b` module
-            "<b,%1> ($1) [root] -> [b.py(0) definition b] <%1> ($1)",
-            // definition of `foo` inside of `b` module
-            "<b.foo,%2> ($1) [root] -> [b.py(5) definition foo] <%2> ($1)",
-        ],
+        crate::can_find_partial_paths_in_file::SEQUENCED_IMPORT_STAR_B_PATHS,
     );
 }