Remove here keyword (#3749)

Changelog
- Remove `HERE` keyword
- Replace `here` with module name when generating code
This commit is contained in:
Dmitry Bushev 2022-09-29 21:34:17 +03:00 committed by GitHub
parent 61a4120cfb
commit 15084feaa6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 53 additions and 49 deletions

View File

@ -66,6 +66,7 @@
- [Selecting a suggestion from the searcher or component browser now updates the
visualisation of the edited node to preview the results of applying the
suggestion.][3691]
- [Remove here keyword from IDE.][3749]
#### EnsoGL (rendering engine)
@ -326,6 +327,7 @@
[3726]: https://github.com/enso-org/enso/pull/3726
[3727]: https://github.com/enso-org/enso/pull/3727
[3733]: https://github.com/enso-org/enso/pull/3733
[3749]: https://github.com/enso-org/enso/pull/3749
#### Enso Compiler

View File

@ -13,7 +13,6 @@ use crate::identifier::ReferentName;
use crate::project;
use crate::tp;
use ast::constants::keywords::HERE;
use ast::constants::PROJECTS_MAIN_MODULE;
use ast::crumbs::ChildAst;
use ast::crumbs::ModuleCrumb;
@ -780,27 +779,22 @@ pub fn locate(
/// The module is assumed to be in the file identified by the `method.file` (for the purpose of
/// desugaring implicit extensions methods for modules).
///
/// The `module_name` parameter is the name of the module that contains `ast`. It affects how the
/// `here` keyword is resolved.
/// The `module_name` parameter is the name of the module that contains `ast`.
pub fn lookup_method(
module_name: &QualifiedName,
ast: &known::Module,
method: &language_server::MethodPointer,
) -> FallibleResult<definition::Id> {
let qualified_typename = tp::QualifiedName::from_text(&method.defined_on_type)?;
let accept_here_methods = module_name == &qualified_typename;
let defined_in_this_module = module_name == &qualified_typename;
let method_module_name = QualifiedName::try_from(method)?;
let implicit_extension_allowed = method.defined_on_type == method_module_name.to_string();
for child in ast.def_iter() {
let child_name = &child.name.item;
let name_matches = child_name.name.item == method.name;
let type_matches = match child_name.extended_target.as_slice() {
[] => implicit_extension_allowed,
[typename] => {
let explicit_type_matching = typename.item == qualified_typename.name;
let here_extension_matching = typename.item == HERE && accept_here_methods;
explicit_type_matching || here_extension_matching
}
[] => implicit_extension_allowed || defined_in_this_module,
[typename] => typename.item == qualified_typename.name,
_ => child_name.explicitly_extends_type(&method.defined_on_type),
};
if name_matches && type_matches {
@ -952,9 +946,6 @@ mod tests {
// Explicit module extension method.
let id = definition::Id::new_single_crumb(DefinitionName::new_method("Main", "foo"));
expect_find(&ptr, "Main.foo a b = a + b", &id);
// Explicit extensions using "here" keyword.
let id = definition::Id::new_single_crumb(DefinitionName::new_method("here", "foo"));
expect_find(&ptr, "here.foo a b = a + b", &id);
// Matching name but extending wrong type.
expect_not_found(&ptr, "Number.foo a b = a + b");
// Mismatched name.
@ -973,7 +964,6 @@ mod tests {
let id = definition::Id::new_single_crumb(DefinitionName::new_method("Number", "foo"));
expect_find(&ptr, "Number.foo a b = a + b", &id);
expect_not_found(&ptr, "Text.foo a b = a + b");
expect_not_found(&ptr, "here.foo a b = a + b");
expect_not_found(&ptr, "bar a b = a + b");
}
@ -1015,7 +1005,7 @@ last def = inline expression";
let parser = parser::Parser::new_or_panic();
let module = r#"Main.method1 arg = body
main = here.method1 10"#;
main = Main.method1 10"#;
let module = Info::from(parser.parse_module(module, default()).unwrap());
let method1_id = DefinitionName::new_method("Main", "method1");
@ -1037,12 +1027,12 @@ main = here.method1 10"#;
Main.method1 arg = body
main = here.method1 10"#;
main = Main.method1 10"#;
assert_eq!(repr_after_insertion(Placement::Begin), expected);
let expected = r#"Main.method1 arg = body
main = here.method1 10
main = Main.method1 10
Main.add arg1 arg2 = arg1 + arg2"#;
assert_eq!(repr_after_insertion(Placement::End), expected);
@ -1051,7 +1041,7 @@ Main.add arg1 arg2 = arg1 + arg2"#;
Main.add arg1 arg2 = arg1 + arg2
main = here.method1 10"#;
main = Main.method1 10"#;
assert_eq!(repr_after_insertion(Placement::After(method1_id.clone())), expected);
assert_eq!(

View File

@ -10,11 +10,11 @@ use crate::definition;
use crate::definition::DefinitionInfo;
use crate::graph::GraphInfo;
use crate::identifier::Identifier;
use crate::identifier::ReferentName;
use crate::node;
use crate::node::MainLine;
use crate::node::NodeInfo;
use ast::constants::keywords::HERE;
use ast::crumbs::Located;
use ast::BlockLine;
use parser::Parser;
@ -41,8 +41,9 @@ pub fn collapse(
selected_nodes: impl IntoIterator<Item = node::Id>,
name: Identifier,
parser: &Parser,
module_name: ReferentName,
) -> FallibleResult<Collapsed> {
Collapser::new(graph.clone(), selected_nodes, parser.clone_ref())?.collapse(name)
Collapser::new(graph.clone(), selected_nodes, parser.clone_ref(), module_name)?.collapse(name)
}
@ -297,6 +298,7 @@ pub struct Collapser {
parser: Parser,
/// Identifier of the node to be introduced as a result of collapsing.
collapsed_node: node::Id,
module_name: ReferentName,
}
impl Collapser {
@ -306,13 +308,14 @@ impl Collapser {
graph: GraphInfo,
selected_nodes: impl IntoIterator<Item = node::Id>,
parser: Parser,
module_name: ReferentName,
) -> FallibleResult<Self> {
let graph = GraphHelper::new(graph);
let extracted = Extracted::new(&graph, selected_nodes)?;
let last_selected = extracted.extracted_nodes.iter().last().ok_or(NoNodesSelected)?.id();
let replaced_node = extracted.output.as_ref().map(|out| out.node).unwrap_or(last_selected);
let collapsed_node = node::Id::new_v4();
Ok(Collapser { graph, extracted, replaced_node, parser, collapsed_node })
Ok(Collapser { graph, extracted, replaced_node, parser, collapsed_node, module_name })
}
/// Generate the expression that calls the extracted method definition.
@ -321,7 +324,7 @@ impl Collapser {
pub fn call_to_extracted(&self, extracted: &definition::ToAdd) -> FallibleResult<Ast> {
// TODO actually check that generated name is single-identifier
let mut target = extracted.name.clone();
target.extended_target.insert(0, Located::new_root(HERE.to_string()));
target.extended_target.insert(0, Located::new_root(self.module_name.clone().into()));
let base = target.ast(&self.parser)?;
let args = extracted.explicit_parameter_names.iter().map(Ast::var);
let chain = ast::prefix::Chain::new(base, args);
@ -389,6 +392,7 @@ mod tests {
use ast::crumbs::Crumb;
struct Case {
module_name: ReferentName,
refactored_name: DefinitionName,
introduced_name: Identifier,
initial_method_code: &'static str,
@ -408,7 +412,8 @@ mod tests {
ast::test_utils::assert_unique_ids(ast.as_ref());
let selection = selection.iter().copied();
let new_name = self.introduced_name.clone();
let collapsed = collapse(&graph, selection, new_name, parser).unwrap();
let module_name = self.module_name.clone();
let collapsed = collapse(&graph, selection, new_name, parser, module_name).unwrap();
let new_method = collapsed.new_method.ast(0, parser).unwrap();
let placement = module::Placement::Before(self.refactored_name.clone());
let new_main = &collapsed.updated_definition.ast;
@ -441,6 +446,7 @@ mod tests {
#[wasm_bindgen_test]
fn test_collapse() {
let parser = Parser::new_or_panic();
let module_name = ReferentName::new("Main".to_owned()).unwrap();
let introduced_name = Identifier::try_from("custom_new").unwrap();
let refactored_name = DefinitionName::new_plain("custom_old");
let initial_method_code = r"custom_old =
@ -457,10 +463,11 @@ mod tests {
c";
let expected_refactored = r"custom_old =
a = 1
c = here.custom_new a
c = Main.custom_new a
c + 7";
let mut case = Case {
module_name,
refactored_name,
introduced_name,
initial_method_code,
@ -483,7 +490,7 @@ mod tests {
a = 1
b = 2
c = A + B
d = here.custom_new a b
d = Main.custom_new a b
c + 7";
case.run(&parser);
@ -502,7 +509,7 @@ mod tests {
a = 1
b = 2
c = A + B
here.custom_new a b
Main.custom_new a b
c + 7";
case.run(&parser);
@ -518,7 +525,7 @@ mod tests {
c = 50 + d
c";
case.expected_refactored = r"custom_old =
c = here.custom_new
c = Main.custom_new
c + c + 10";
case.run(&parser);
@ -537,7 +544,7 @@ mod tests {
case.expected_refactored = r"custom_old =
number1 = 1
number2 = 2
vector = here.custom_new number1 number2";
vector = Main.custom_new number1 number2";
case.run(&parser);
}
}

View File

@ -65,8 +65,6 @@ pub mod constants {
/// A module with language-specific constants.
pub mod keywords {
/// A keyword indicating current module.
pub const HERE: &str = "here";
/// The "void" atom returned by function meant to not return any argument.
pub const NOTHING: &str = "Nothing";

View File

@ -897,7 +897,8 @@ impl Handle {
let introduced_name = module.generate_name(new_method_name_base)?;
let node_ids = nodes.iter().map(|node| node.info.id());
let graph = self.graph_info()?;
let collapsed = collapse(&graph, node_ids, introduced_name, &self.parser)?;
let module_name = self.module.name();
let collapsed = collapse(&graph, node_ids, introduced_name, &self.parser, module_name)?;
let Collapsed { new_method, updated_definition, collapsed_node } = collapsed;
let graph = self.graph_info()?;
@ -1275,7 +1276,7 @@ func3 a =
main =
a = 10
here.func3 a
Mock_Module.func3 a
a + func1";
test.data.code = code.to_owned();
@ -1303,7 +1304,7 @@ func1 =
a
main =
a = here.func1
a = Mock_Module.func1
a + c";
test.data.code = code.to_owned();

View File

@ -302,7 +302,6 @@ mod tests {
assert_eq!(code, module.ast().repr());
};
expect_intact("main = 5");
expect_intact("here.main = 5");
expect_intact(&format!("{}.main = 5", module_name));
}
}

View File

@ -955,9 +955,10 @@ impl Searcher {
// === Add new node ===
let here = Ast::var(ast::constants::keywords::HERE);
let args = std::iter::empty();
let node_expression = ast::prefix::Chain::new_with_this(new_definition_name, here, args);
let this_expression = Ast::var(self.module_qualified_name().name());
let node_expression =
ast::prefix::Chain::new_with_this(new_definition_name, this_expression, args);
let node_expression = node_expression.into_ast();
let node = NodeInfo::from_main_line_ast(&node_expression).ok_or(FailedToCreateNode)?;
let added_node_id = node.id();
@ -1247,9 +1248,7 @@ impl Searcher {
self.database.lookup_locals_by_name_and_location(this_name, &module_name, position);
let not_local_name = matching_locals.is_empty();
not_local_name.and_option_from(|| {
if this_name == ast::constants::keywords::HERE
|| this_name == module_name.name().deref()
{
if this_name == module_name.name().deref() {
Some(module_name)
} else {
self.module().iter_imports().find_map(|import| {
@ -1955,14 +1954,12 @@ pub mod test {
data.expect_completion(client, None, Some("String"), &[1]);
data.expect_completion(client, None, Some("Number"), &[]);
data.expect_completion(client, None, Some("Number"), &[]);
data.expect_completion(client, None, Some("Number"), &[]);
data.expect_completion(client, None, None, &[1, 2, 3, 4, 9]);
});
let Fixture { searcher, .. } = &mut fixture;
// Known functions cases
searcher.set_input("Test.testMethod1 ".to_string()).unwrap();
searcher.set_input("here.testMethod1 ".to_string()).unwrap();
searcher.set_input(iformat!("{MODULE_NAME}.testMethod1 ")).unwrap();
searcher.set_input("testFunction2 \"str\" ".to_string()).unwrap();
@ -2518,7 +2515,7 @@ pub mod test {
documentation_html: "Lorem ipsum".to_owned(),
};
let expected_code = "test_example1 =\n x = 2 + 2\n x + 4\n\n\
main = \n 2 + 2\n here.test_example1";
main = \n 2 + 2\n Mock_Module.test_example1";
searcher.add_example(&Rc::new(example)).unwrap();
assert_eq!(module.ast().repr(), expected_code);
}
@ -2535,7 +2532,7 @@ pub mod test {
};
let expected_code = "import std.Base.Network.Http\n\
test_example1 = [1,2,3,4,5]\n\ntest_example2 = [1,2,3,4,5]\n\n\
main = \n 2 + 2\n here.test_example1\n here.test_example2";
main = \n 2 + 2\n Mock_Module.test_example1\n Mock_Module.test_example2";
let example = Rc::new(example);
searcher.add_example(&example).unwrap();
searcher.add_example(&example).unwrap();

View File

@ -550,6 +550,11 @@ pub trait API: Debug + model::undo_redo::Aware {
/// Get the module path.
fn path(&self) -> &Path;
/// Get the module name.
fn name(&self) -> ReferentName {
self.path().module_name()
}
/// Get module sources as a string, which contains both code and metadata.
fn serialized_content(&self) -> FallibleResult<SourceFile>;

View File

@ -4,7 +4,6 @@ use crate::prelude::*;
use crate::model::module::MethodId;
use ast::constants::keywords;
use convert_case::Case;
use convert_case::Casing;
use double_representation::module;
@ -261,14 +260,14 @@ impl Entry {
imports.insert(self.module.clone());
}
let this_expr = if generate_this {
let this_expr: Option<String> = if generate_this {
// TODO [mwu] Currently we support `self` generation for module atoms only.
// This should be extended to any atom that is known to be nullary.
// Tracked by https://github.com/enso-org/ide/issues/1299
if self.is_regular_module_method() {
if is_local_entry {
// No additional import for `here`.
Some(keywords::HERE.to_owned())
// No additional import for entries defined in this module.
Some(self.module.name().into())
} else {
// If we are inserting an additional `self` argument, the used name must be
// visible.
@ -791,7 +790,7 @@ mod test {
expect(&module_method, None, true, "Project.module_method", &[&main_module]);
expect(&module_method, None, false, "module_method", &[]);
expect(&module_method, Some(&main_module), true, "here.module_method", &[]);
expect(&module_method, Some(&main_module), true, "Main.module_method", &[]);
expect(&module_method, Some(&main_module), false, "module_method", &[]);
expect(&module_method, Some(&another_module), true, "Project.module_method", &[
&main_module,
@ -810,7 +809,13 @@ mod test {
&[&another_module],
);
expect(&another_module_method, Some(&main_module), false, "module_method", &[]);
expect(&another_module_method, Some(&another_module), true, "here.module_method", &[]);
expect(
&another_module_method,
Some(&another_module),
true,
"Another_Module.module_method",
&[],
);
expect(&another_module_method, Some(&another_module), false, "module_method", &[]);
// TODO [mwu] Extensions on nullary atoms should also be able to generate this.