Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove here keyword #3749

Merged
merged 9 commits into from
Sep 29, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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

Expand Down
9 changes: 3 additions & 6 deletions app/gui/controller/double-representation/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -780,15 +779,14 @@ 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() {
Expand All @@ -798,8 +796,7 @@ pub fn lookup_method(
[] => 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
explicit_type_matching || defined_in_this_module
}
_ => child_name.explicitly_extends_type(&method.defined_on_type),
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
}


Expand Down Expand Up @@ -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 {
Expand All @@ -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.
Expand All @@ -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);
Expand Down Expand Up @@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -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 =
Expand All @@ -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,
Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);
}
}
2 changes: 0 additions & 2 deletions app/gui/language/ast/impl/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
3 changes: 2 additions & 1 deletion app/gui/src/controller/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()?;
Expand Down
9 changes: 4 additions & 5 deletions app/gui/src/controller/searcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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| {
Expand Down
3 changes: 3 additions & 0 deletions app/gui/src/model/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,9 @@ pub trait API: Debug + model::undo_redo::Aware {
/// Get the module path.
fn path(&self) -> &Path;

/// Get the module name.
fn name(&self) -> ReferentName;

/// Get module sources as a string, which contains both code and metadata.
fn serialized_content(&self) -> FallibleResult<SourceFile>;

Expand Down
5 changes: 5 additions & 0 deletions app/gui/src/model/module/plain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use crate::model::module::TextChange;
use crate::notification;

use double_representation::definition::DefinitionInfo;
use double_representation::identifier::ReferentName;
use double_representation::module::ImportId;
use flo_stream::Subscriber;
use parser::api::ParsedSourceFile;
Expand Down Expand Up @@ -136,6 +137,10 @@ impl model::module::API for Module {
&self.path
}

fn name(&self) -> ReferentName {
self.path.module_name()
}
4e6 marked this conversation as resolved.
Show resolved Hide resolved

fn serialized_content(&self) -> FallibleResult<SourceFile> {
self.content.borrow().serialize().map_err(|e| e.into())
}
Expand Down
5 changes: 5 additions & 0 deletions app/gui/src/model/module/synchronized.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use crate::model::module::API;
use ast::IdMap;
use double_representation::definition::DefinitionInfo;
use double_representation::graph::Id;
use double_representation::identifier::ReferentName;
use double_representation::module::ImportId;
use engine_protocol::language_server;
use engine_protocol::language_server::TextEdit;
Expand Down Expand Up @@ -194,6 +195,10 @@ impl API for Module {
self.model.path()
}

fn name(&self) -> ReferentName {
self.path().module_name()
}

fn serialized_content(&self) -> FallibleResult<SourceFile> {
self.model.serialized_content()
}
Expand Down
17 changes: 11 additions & 6 deletions app/gui/src/model/suggestion_database/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand All @@ -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.
Expand Down