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

Implement renaming for Functions, Constants and Type Variants #4282

Draft
wants to merge 27 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
058daff
Find in-module references to constants and functions
GearsDatapacks Feb 24, 2025
e43157b
Implement renaming for local constants and functions
GearsDatapacks Feb 24, 2025
c2edde6
Fix rename location
GearsDatapacks Feb 24, 2025
9cabeef
Implement cross-module renaming in language server
GearsDatapacks Feb 25, 2025
d59eedf
Move usage detection to analyser
GearsDatapacks Feb 25, 2025
e91923a
Properly rename definition name
GearsDatapacks Feb 25, 2025
b543848
Fix cross-module renaming
GearsDatapacks Feb 25, 2025
e5b0c2e
Register usage on unqualified imports
GearsDatapacks Feb 25, 2025
6cd0e50
Clippy
GearsDatapacks Feb 25, 2025
5c1c9bb
Update snapshots
GearsDatapacks Feb 25, 2025
65851a9
Changelog
GearsDatapacks Feb 25, 2025
922529b
Fix renaming from the outside module a value is defined in
GearsDatapacks Feb 25, 2025
88eea05
Add more renaming locations for constants and functions
GearsDatapacks Feb 25, 2025
86c8e51
Allow renaming of type variants
GearsDatapacks Feb 26, 2025
bcb7977
Fix renaming of non-name parts of definitions
GearsDatapacks Feb 26, 2025
8fd34bf
Don't rename aliased unqualified imports
GearsDatapacks Feb 26, 2025
426cd38
Fix from_file_path for wasm
GearsDatapacks Feb 26, 2025
c59b29c
Add tests
GearsDatapacks Feb 26, 2025
4d98cc9
Fix bug with module access shadowing
GearsDatapacks Feb 28, 2025
ce9ba66
Clippy
GearsDatapacks Feb 28, 2025
d81edf4
Format
GearsDatapacks Feb 28, 2025
71ac89d
Add failing tests
lpil Mar 1, 2025
fd39b57
Implement renaming in patterns
GearsDatapacks Mar 1, 2025
6e5051b
Address style comments
GearsDatapacks Mar 1, 2025
1afbf2b
Cache reference information
GearsDatapacks Mar 2, 2025
5e00fad
Add call graph to reference tracking
GearsDatapacks Mar 9, 2025
d5aed34
Clippy
GearsDatapacks Mar 10, 2025
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
34 changes: 34 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,40 @@

### Language server

- The language server now allows renaming of functions and constants across modules.
For example:

```gleam
// wibble.gleam
pub fn wibble() {
wibble()
//^ Trigger rename
}
// wobble.gleam
import wibble

pub fn main() {
wibble.wibble()
}
```

Becomes:

```gleam
// wibble.gleam
pub fn wobble() {
wobble()
}
// wobble.gleam
import wibble

pub fn main() {
wibble.wobble()
}
```

([Surya Rose](https://github.com/GearsDatapacks))

### Formatter

### Bug fixes
Expand Down
1,211 changes: 902 additions & 309 deletions compiler-core/generated/schema_capnp.rs

Large diffs are not rendered by default.

13 changes: 13 additions & 0 deletions compiler-core/schema.capnp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,18 @@ struct Module {
typeAliases @10 :List(Property(TypeAliasConstructor));
documentation @11 :List(Text);
containsEcho @12 :Bool;
references @13 :References;
}

struct References {
importedModules @0 :List(Text);
valueReferences @1 :List(ValueReference);
}

struct ValueReference {
module @0 :Text;
name @1 :Text;
references @2 :List(SrcSpan);
}

struct TypeAliasConstructor {
Expand Down Expand Up @@ -161,6 +173,7 @@ struct ValueConstructorVariant {
literal @0 :Constant;
location @1 :SrcSpan;
module @2 :Text;
name @22 :Text;
documentation @14 :Text;
implementations @19 :Implementations;
}
Expand Down
32 changes: 30 additions & 2 deletions compiler-core/src/analyse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ use crate::{
parse::SpannedString,
type_::{
self, AccessorsMap, Deprecation, ModuleInterface, Opaque, PatternConstructor,
RecordAccessor, Type, TypeAliasConstructor, TypeConstructor, TypeValueConstructor,
TypeValueConstructorField, TypeVariantConstructors, ValueConstructor,
RecordAccessor, References, Type, TypeAliasConstructor, TypeConstructor,
TypeValueConstructor, TypeValueConstructorField, TypeVariantConstructors, ValueConstructor,
ValueConstructorVariant, Warning,
environment::*,
error::{Error, FeatureKind, MissingAnnotation, Named, Problems, convert_unify_error},
Expand Down Expand Up @@ -340,6 +340,10 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
type_aliases,
documentation,
contains_echo: echo_found,
references: References {
imported_modules: env.imported_modules.into_keys().collect(),
value_references: env.references.into_locations(),
},
},
names: type_names,
};
Expand Down Expand Up @@ -407,6 +411,7 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
location,
literal: typed_expr.clone(),
module: self.module_name.clone(),
name: name.clone(),
implementations,
},
type_: type_.clone(),
Expand All @@ -421,6 +426,12 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
);
environment.insert_module_value(name.clone(), variant);

environment.references.register_reference(
environment.current_module.clone(),
name.clone(),
name_location,
);

if publicity.is_private() {
environment.init_usage(
name.clone(),
Expand Down Expand Up @@ -516,6 +527,10 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
.map(|(a, t)| a.set_type(t.clone()))
.collect_vec();

environment
.references
.enter_function(environment.current_module.clone(), name.clone());

// Infer the type using the preregistered args + return types as a starting point
let result = environment.in_new_scope(&mut self.problems, |environment, problems| {
let mut expr_typer = ExprTyper::new(environment, definition, problems);
Expand Down Expand Up @@ -634,6 +649,12 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
deprecation.clone(),
);

environment.references.register_reference(
environment.current_module.clone(),
name.clone(),
name_location,
);

Definition::Function(Function {
documentation: doc,
location,
Expand Down Expand Up @@ -1079,6 +1100,12 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
},
);

environment.references.register_reference(
environment.current_module.clone(),
constructor.name.clone(),
constructor.name_location,
);

if value_constructor_publicity.is_private() {
environment.init_usage(
constructor.name.clone(),
Expand Down Expand Up @@ -1624,6 +1651,7 @@ fn generalise_module_constant(
literal: *value.clone(),
module: module_name.clone(),
implementations,
name: name.clone(),
};
environment.insert_variable(
name.clone(),
Expand Down
19 changes: 19 additions & 0 deletions compiler-core/src/analyse/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,25 @@ impl<'context, 'problems> Importer<'context, 'problems> {
name.clone(),
used_name.clone(),
);
self.environment.references.register_reference(
module.clone(),
import_name.clone(),
import.imported_name_location,
);
}
ValueConstructorVariant::ModuleConstant { module, .. }
| ValueConstructorVariant::ModuleFn { module, .. } => {
self.environment.init_usage(
used_name.clone(),
EntityKind::ImportedValue,
location,
self.problems,
);
self.environment.references.register_reference(
module.clone(),
import_name.clone(),
import.imported_name_location,
);
}
_ => self.environment.init_usage(
used_name.clone(),
Expand Down
24 changes: 14 additions & 10 deletions compiler-core/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -892,20 +892,21 @@ impl TypedDefinition {

Definition::CustomType(custom) => {
// Check if location is within the type of one of the arguments of a constructor.
if let Some(annotation) = custom
if let Some(constructor) = custom
.constructors
.iter()
.find(|constructor| constructor.location.contains(byte_index))
.and_then(|constructor| {
constructor
.arguments
.iter()
.find(|arg| arg.location.contains(byte_index))
})
.filter(|arg| arg.location.contains(byte_index))
.and_then(|arg| arg.ast.find_node(byte_index, arg.type_.clone()))
{
return Some(annotation);
if let Some(annotation) = constructor
.arguments
.iter()
.find(|arg| arg.location.contains(byte_index))
.and_then(|arg| arg.ast.find_node(byte_index, arg.type_.clone()))
{
return Some(annotation);
}

return Some(Located::VariantConstructorDefinition(constructor));
}

// Note that the custom type `.location` covers the function
Expand Down Expand Up @@ -1077,6 +1078,8 @@ impl<A, B, C, E> Definition<A, B, C, E> {
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct UnqualifiedImport {
pub location: SrcSpan,
/// The location excluding the potential `as ...` clause, or the `type` keyword
pub imported_name_location: SrcSpan,
pub name: EcoString,
pub as_name: Option<EcoString>,
}
Expand Down Expand Up @@ -1819,6 +1822,7 @@ pub enum Pattern<Type> {
/// The constructor for a custom type. Starts with an uppercase letter.
Constructor {
location: SrcSpan,
name_location: SrcSpan,
name: EcoString,
arguments: Vec<CallArg<Self>>,
module: Option<(EcoString, SrcSpan)>,
Expand Down
5 changes: 5 additions & 0 deletions compiler-core/src/ast/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,7 @@ pub trait Visit<'ast> {
fn visit_typed_pattern_constructor(
&mut self,
location: &'ast SrcSpan,
name_location: &'ast SrcSpan,
name: &'ast EcoString,
arguments: &'ast Vec<CallArg<TypedPattern>>,
module: &'ast Option<(EcoString, SrcSpan)>,
Expand All @@ -479,6 +480,7 @@ pub trait Visit<'ast> {
visit_typed_pattern_constructor(
self,
location,
name_location,
name,
arguments,
module,
Expand Down Expand Up @@ -1473,6 +1475,7 @@ where
} => v.visit_typed_pattern_list(location, elements, tail, type_),
Pattern::Constructor {
location,
name_location,
name,
arguments,
module,
Expand All @@ -1481,6 +1484,7 @@ where
type_,
} => v.visit_typed_pattern_constructor(
location,
name_location,
name,
arguments,
module,
Expand Down Expand Up @@ -1593,6 +1597,7 @@ pub fn visit_typed_pattern_list<'a, V>(
pub fn visit_typed_pattern_constructor<'a, V>(
v: &mut V,
_location: &'a SrcSpan,
_name_location: &'a SrcSpan,
_name: &'a EcoString,
arguments: &'a Vec<CallArg<TypedPattern>>,
_module: &'a Option<(EcoString, SrcSpan)>,
Expand Down
14 changes: 13 additions & 1 deletion compiler-core/src/ast_folder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1173,13 +1173,21 @@ pub trait PatternFolder {

Pattern::Constructor {
location,
name_location,
name,
arguments,
module,
spread,
constructor: _,
type_: (),
} => self.fold_pattern_constructor(location, name, arguments, module, spread),
} => self.fold_pattern_constructor(
location,
name_location,
name,
arguments,
module,
spread,
),

Pattern::Tuple { location, elems } => self.fold_pattern_tuple(location, elems),

Expand Down Expand Up @@ -1289,13 +1297,15 @@ pub trait PatternFolder {
fn fold_pattern_constructor(
&mut self,
location: SrcSpan,
name_location: SrcSpan,
name: EcoString,
arguments: Vec<CallArg<UntypedPattern>>,
module: Option<(EcoString, SrcSpan)>,
spread: Option<SrcSpan>,
) -> UntypedPattern {
Pattern::Constructor {
location,
name_location,
name,
arguments,
module,
Expand Down Expand Up @@ -1390,6 +1400,7 @@ pub trait PatternFolder {

Pattern::Constructor {
location,
name_location,
name,
arguments,
module,
Expand All @@ -1406,6 +1417,7 @@ pub trait PatternFolder {
.collect();
Pattern::Constructor {
location,
name_location,
name,
arguments,
module,
Expand Down
10 changes: 8 additions & 2 deletions compiler-core/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ pub use self::project_compiler::{Built, Options, ProjectCompiler};
pub use self::telemetry::{NullTelemetry, Telemetry};

use crate::ast::{
self, CallArg, CustomType, DefinitionLocation, Pattern, TypeAst, TypedArg, TypedDefinition,
TypedExpr, TypedFunction, TypedPattern, TypedStatement,
self, CallArg, CustomType, DefinitionLocation, TypedArg, TypedDefinition, TypedExpr,
TypedFunction, TypedPattern, TypedRecordConstructor, TypedStatement,
};
use crate::type_::Type;
use crate::{
Expand Down Expand Up @@ -339,6 +339,7 @@ pub enum Located<'a> {
Statement(&'a TypedStatement),
Expression(&'a TypedExpr),
ModuleStatement(&'a TypedDefinition),
VariantConstructorDefinition(&'a TypedRecordConstructor),
FunctionBody(&'a TypedFunction),
Arg(&'a TypedArg),
Annotation(SrcSpan, std::sync::Arc<Type>),
Expand Down Expand Up @@ -382,6 +383,10 @@ impl<'a> Located<'a> {
module: None,
span: statement.location(),
}),
Self::VariantConstructorDefinition(record) => Some(DefinitionLocation {
module: None,
span: record.location,
}),
Self::UnqualifiedImport(UnqualifiedImport {
module,
name,
Expand Down Expand Up @@ -420,6 +425,7 @@ impl<'a> Located<'a> {

Located::PatternSpread { .. } => None,
Located::ModuleStatement(definition) => None,
Located::VariantConstructorDefinition(_) => None,
Located::FunctionBody(function) => None,
Located::UnqualifiedImport(unqualified_import) => None,
Located::ModuleName { .. } => None,
Expand Down
1 change: 1 addition & 0 deletions compiler-core/src/build/package_loader/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ fn write_cache(
type_aliases: Default::default(),
documentation: Default::default(),
contains_echo: false,
references: Default::default(),
};
let path = Utf8Path::new("/artefact").join(format!("{name}.cache"));
fs.write_bytes(
Expand Down
Loading
Loading