Don't insert imports when method calls are created through searcher

CHANGELOG.md

@@ -22,6 +22,9 @@ read the notes of the `Enso 2.0.0-alpha.1` release.
 - [Assigning intermediate expressions to values][1067]. Nodes added with searcher will have their 
   values automatically assigned to a newly generated variables. This allows Enso Engine to cache
   intermediate values, improving visualization performance.
+- [Not adding spurious imports][1209]. Fixed cases when IDE was adding unnecessary library 
+  imports when selecting hints from node searcher. This makes the generated textual code easier to
+  read and reduces likelihood of accidental name collision.
 
 #### EnsoGL
 - New multi-camera management system, allowing the same shape systems be rendered on different 
@@ -55,6 +58,8 @@ read the notes of the `Enso 2.0.0-alpha.1` release.
 [1160]: https://github.com/enso-org/ide/pull/1160
 [1190]: https://github.com/enso-org/ide/pull/1190
 [1187]: https://github.com/enso-org/ide/pull/1187
+[1067]: https://github.com/enso-org/ide/issues/1067
+[1209]: https://github.com/enso-org/ide/pull/1209
 <br/>
 
 

src/rust/ide/src/controller/searcher.rs

@@ -733,11 +733,11 @@ impl Searcher {
     fn add_required_imports(&self) -> FallibleResult {
         let data_borrowed = self.data.borrow();
         let fragments     = data_borrowed.fragments_added_by_picking.iter();
-        let imports       = fragments.map(|frag| &frag.picked_suggestion.module);
+        let imports       = fragments.filter_map(|frag| frag.picked_suggestion.required_import());
         let mut module    = self.module();
         let here          = self.module_qualified_name();
         for import in imports {
-            module.add_module_import(&here, &self.parser, &import);
+            module.add_module_import(&here, &self.parser, import);
         }
         self.graph.graph().module.update_ast(module.ast)
     }
@@ -1067,6 +1067,7 @@ pub mod test {
         entry3   : action::Suggestion,
         entry4   : action::Suggestion,
         entry9   : action::Suggestion,
+        entry10  : action::Suggestion,
     }
 
     impl Fixture {
@@ -1190,6 +1191,12 @@ pub mod test {
                 ],
                 ..entry1.clone()
             };
+            let entry10 = model::suggestion_database::Entry {
+                name   : "testFunction3".to_string(),
+                module : "Test.Other".to_owned().try_into().unwrap(),
+                scope  : Scope::Everywhere,
+                ..entry9.clone()
+            };
 
             searcher.database.put_entry(1,entry1);
             let entry1 = searcher.database.lookup(1).unwrap();
@@ -1201,7 +1208,9 @@ pub mod test {
             let entry4 = searcher.database.lookup(4).unwrap();
             searcher.database.put_entry(9,entry9);
             let entry9 = searcher.database.lookup(9).unwrap();
-            Fixture{data,test,searcher,entry1,entry2,entry3,entry4,entry9}
+            searcher.database.put_entry(10,entry10);
+            let entry10 = searcher.database.lookup(10).unwrap();
+            Fixture{data,test,searcher,entry1,entry2,entry3,entry4,entry9,entry10}
         }
 
         fn new() -> Self {
@@ -1601,6 +1610,53 @@ pub mod test {
         }
     }
 
+    #[wasm_bindgen_test]
+    fn adding_imports_with_nodes() {
+        let fixture = Fixture::new();
+        let Fixture{test:_test,mut searcher,entry1,entry2,entry3,entry4,entry10,..} = fixture;
+
+        assert!(entry1.required_import().is_some());
+        assert!(entry2.required_import().is_some());
+        assert!(entry3.required_import().is_none()); // No imports needed, as it is a method.
+        assert!(entry4.required_import().is_none()); // No imports needed, as it is a method.
+        assert!(entry10.required_import().is_some());
+
+        let module      = searcher.graph.graph().module.clone_ref();
+        let initial_ast = module.ast();
+        let parser      = searcher.parser.clone_ref();
+
+        // Helper that returns import that got added when committing a node from given entry.
+        let mut import_added_for = |entry:action::Suggestion| {
+            let picked_method = FragmentAddedByPickingSuggestion {
+                id                : CompletedFragmentId::Function,
+                picked_suggestion : entry.clone(),
+            };
+            with(searcher.data.borrow_mut(), |mut data| {
+                data.fragments_added_by_picking.push(picked_method);
+                data.input = ParsedInput::new(entry.name.to_string(),&parser).unwrap();
+            });
+
+            // Add new node.
+            searcher.mode = Immutable(Mode::NewNode {position:None});
+            searcher.commit_node().unwrap();
+
+            let module_info = module.info();
+            let mut imports = module_info.iter_imports();
+            let ret = imports.next();
+            assert!(imports.next().is_none()); // at most one import insertion is expected
+            println!("{}",module.ast());
+            module.update_ast(initial_ast.clone()).unwrap();
+            ret
+        };
+
+        assert_eq!(import_added_for(entry1), None); // No import added, as we are the relevant module.
+        assert_eq!(import_added_for(entry2), None); // No import added, as we are the relevant module.
+        assert_eq!(import_added_for(entry3), None); // No import added, as this is a method entry.
+        assert_eq!(import_added_for(entry4), None); // No import added, as this is a method entry.
+        let import10 = import_added_for(entry10.clone_ref()).unwrap();
+        assert_eq!(import10.qualified_name().unwrap(), entry10.module);
+    }
+
     #[wasm_bindgen_test]
     fn committing_node() {
         let Fixture{test:_test,mut searcher,entry4,..} = Fixture::new();
@@ -1621,7 +1677,7 @@ pub mod test {
         searcher.mode = Immutable(Mode::NewNode {position});
         searcher.commit_node().unwrap();
 
-        let expected_code = "import Test.Test\nmain = \n    2 + 2\n    operator1 = Test.testMethod1";
+        let expected_code = "main = \n    2 + 2\n    operator1 = Test.testMethod1";
         assert_eq!(module.ast().repr(), expected_code);
         let (node1,node2) = searcher.graph.graph().nodes().unwrap().expect_tuple();
         let expected_intended_method = Some(MethodId {
@@ -1634,7 +1690,7 @@ pub mod test {
         // Edit existing node.
         searcher.mode = Immutable(Mode::EditNode {node_id:node1.info.id()});
         searcher.commit_node().unwrap();
-        let expected_code = "import Test.Test\nmain = \n    Test.testMethod1\n    operator1 = Test.testMethod1";
+        let expected_code = "main = \n    Test.testMethod1\n    operator1 = Test.testMethod1";
         let (node1,_)     = searcher.graph.graph().nodes().unwrap().expect_tuple();
         assert_eq!(node1.metadata.unwrap().intended_method, expected_intended_method);
         assert_eq!(module.ast().repr(), expected_code);

src/rust/ide/src/model/suggestion_database/entry.rs

@@ -114,6 +114,16 @@ impl Entry {
         }
     }
 
+    /// Module that needs to be imported to bring in this entry into the scope.
+    pub fn required_import(&self) -> Option<&module::QualifiedName> {
+        if self.kind == Kind::Method {
+            // Methods are found through lookup on `this`, no need for any additional imports.
+            None
+        } else {
+            Some(&self.module)
+        }
+    }
+
     /// Returns the code which should be inserted to Searcher input when suggestion is picked,
     /// omitting module name.
     pub fn code_to_insert_skip_module(&self) -> String {