[move-ide] On-hover improvements #19437
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
awelc
requested review from
cgswords,
damirka,
rvantonder,
dariorussi and
tnowacki
| #[derive(Copy, Clone, Debug)] | ||
| pub enum ModuleMemberKind { | ||
| Constant, | ||
| Function, | ||
| Struct, | ||
| Enum, | ||
| } | ||
|
|
||
| impl ModuleMemberKind { | ||
| pub fn case(self) -> NameCase { | ||
| match self { | ||
| ModuleMemberKind::Constant => NameCase::Constant, | ||
| ModuleMemberKind::Function => NameCase::Function, | ||
| ModuleMemberKind::Struct => NameCase::Struct, | ||
| ModuleMemberKind::Enum => NameCase::Enum, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| #[derive(Copy, Clone, Debug)] | ||
| pub enum NameCase { | ||
| Constant, | ||
| Function, | ||
| Struct, | ||
| Enum, | ||
| Module, | ||
| ModuleMemberAlias(ModuleMemberKind), | ||
| ModuleAlias, | ||
| Variable, | ||
| Address, | ||
| TypeParameter, | ||
| } | ||
|
|
||
| impl NameCase { | ||
| pub const fn name(&self) -> &'static str { | ||
| match self { | ||
| NameCase::Constant => "constant", | ||
| NameCase::Function => "function", | ||
| NameCase::Struct => "struct", | ||
| NameCase::Enum => "enum", | ||
| NameCase::Module => "module", | ||
| NameCase::ModuleMemberAlias(ModuleMemberKind::Function) => "function alias", | ||
| NameCase::ModuleMemberAlias(ModuleMemberKind::Constant) => "constant alias", | ||
| NameCase::ModuleMemberAlias(ModuleMemberKind::Struct) => "struct alias", | ||
| NameCase::ModuleMemberAlias(ModuleMemberKind::Enum) => "enum alias", | ||
| NameCase::ModuleAlias => "module alias", | ||
| NameCase::Variable => "variable", | ||
| NameCase::Address => "address", | ||
| NameCase::TypeParameter => "type parameter", | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm not sure if these should live in the AST. It might be worth making a new module called name_validation that has these plus the checking functions. Then at least all the code and definitions will still live together.
There was a problem hiding this comment.
+1 to these not being in the AST. You could stick them in expansion/mod.rs if you are really struggling for a place. But something like name_validation with all of the other various checking names (some of which might have been dragged into the AST already?) sounds like a great long term solution
There was a problem hiding this comment.
Done, though it required a bit more massaging as all these name validation functions take context as an argument to generate diagnostics
| // Implicit aliases for the Move Stdlib: | ||
| // use std::vector; | ||
| // use std::option::{Self, Option}; | ||
| pub const IMPLICIT_STD_MODULES: &[Symbol] = &[symbol!("option"), symbol!("vector")]; | ||
| pub const IMPLICIT_STD_MEMBERS: &[(Symbol, Symbol, ModuleMemberKind)] = &[( | ||
| symbol!("option"), | ||
| symbol!("Option"), | ||
| ModuleMemberKind::Struct, | ||
| )]; | ||
|
|
||
| // Implicit aliases for Sui mode: | ||
| // use sui::object::{Self, ID, UID}; | ||
| // use sui::transfer; | ||
| // use sui::tx_context::{Self, TxContext}; | ||
| pub const IMPLICIT_SUI_MODULES: &[Symbol] = &[ | ||
| symbol!("object"), | ||
| symbol!("transfer"), | ||
| symbol!("tx_context"), | ||
| ]; | ||
| pub const IMPLICIT_SUI_MEMBERS: &[(Symbol, Symbol, ModuleMemberKind)] = &[ | ||
| (symbol!("object"), symbol!("ID"), ModuleMemberKind::Struct), | ||
| (symbol!("object"), symbol!("UID"), ModuleMemberKind::Struct), | ||
| ( | ||
| symbol!("tx_context"), | ||
| symbol!("TxContext"), | ||
| ModuleMemberKind::Struct, | ||
| ), | ||
| ]; |
There was a problem hiding this comment.
Consider implicit_imports.rs or similar for these definitions, because they don't really belong in the AST file.
There was a problem hiding this comment.
Yeah, a strange one.
We currently have Flavor defined in editions/mod.rs but maybe we need a flavor module. I think default implementations would make a lot of sense in that module, since the default imports are defined by the flavor
There was a problem hiding this comment.
Not sure if there is a concrete suggestion here on where to move it in this PR or it's something we are thinking about but could wait for a separate PR...
| @@ -90,10 +90,13 @@ use move_command_line_common::files::FileHash; | |||
| use move_compiler::{ | |||
There was a problem hiding this comment.
These changes all seem coherent, though I wonder if there is an opportunity to write a helper or two to simplify some of the code.
There was a problem hiding this comment.
I was thinking the same thing, but honestly I could not find a sensible way to helper-ize it as most additional are not easily separable from their enclosing context. Happy to follow suggestions, though!
| @@ -1,6 +1,6 @@ | |||
| module Symbols::M1 { | |||
|
|
|||
| struct SomeStruct has key, drop, store { | |||
There was a problem hiding this comment.
These changes are unrelated, but remove compilation errors which do not affect symbolication but are annoying when working with these Move files
| //************************************************************************************************** | ||
| // Valid names | ||
| //************************************************************************************************** | ||
|
|
There was a problem hiding this comment.
It's strange to me to change these all to Result<(),Box<Diagnostic>> from their original interface. Instead they should take CompilationEnv and retain their previous behavior.
Description
This PR improves on several instance of on-hover.
vector::singletoninstead ofstd::vector::singleton) or both package and modules (for implicit datatypes, e.g.,Option, instead ofstd::option::Option)Test plan
All new and old tests must pass