Skip to content

Commit

Permalink
[move][ide] Report missing match arms as IDE annotations (#17984)
Browse files Browse the repository at this point in the history
## Description 

This reports missing match arms as IDE annotations, so the IDE can
suggest adding them. It only does the outer layer, a bit like Rust.

Note that if an inner pattern is not exhaustive, it will simply report
that as an error, no suggestion (on parity with rust analyzer).

## Test plan 

This is currently not tested. I plan to put up another diff to allow us
to test all IDE annotations by reporting them as diagnostics when in
test mode. When I do that, I will also write tests for this diff.

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
  • Loading branch information
cgswords authored May 30, 2024
1 parent 563ae30 commit 1f147ed
Show file tree
Hide file tree
Showing 65 changed files with 1,350 additions and 114 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ impl CompilerInfo {
eprintln!("Repeated autocomplete info");
}
}
CI::IDEAnnotation::MissingMatchArms(_) => {
// TODO: Not much to do with this yet.
}
}
}
}
Expand Down
16 changes: 11 additions & 5 deletions external-crates/move/crates/move-compiler/src/diagnostics/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ use crate::shared::FILTER_ALL;

#[derive(PartialEq, Eq, Clone, Copy, Debug, Hash, PartialOrd, Ord)]
pub enum Severity {
Warning = 0,
NonblockingError = 1,
BlockingError = 2,
Bug = 3,
Note = 0,
Warning = 1,
NonblockingError = 2,
BlockingError = 3,
Bug = 4,
}

/// A an optional prefix to distinguish between different types of warnings (internal vs. possibly
Expand Down Expand Up @@ -376,7 +377,10 @@ codes!(
AddressAdd: { msg: "move 2024 migration: address add", severity: NonblockingError },
],
IDE: [
Autocomplete: { msg: "IDE autocomplete", severity: NonblockingError },
Autocomplete: { msg: "IDE autocomplete", severity: Note },
MacroCallInfo: { msg: "IDE macro call info", severity: Note },
ExpandedLambda: { msg: "IDE expanded lambda", severity: Note },
MissingMatchArms: { msg: "IDE missing match arms", severity: Note },
],
);

Expand Down Expand Up @@ -435,6 +439,7 @@ impl DiagnosticInfo {
let sev_prefix = match severity {
Severity::BlockingError | Severity::NonblockingError => "E",
Severity::Warning => "W",
Severity::Note => "I",
Severity::Bug => "ICE",
};
debug_assert!(category <= 99);
Expand Down Expand Up @@ -490,6 +495,7 @@ impl Severity {
Severity::Bug => CSRSeverity::Bug,
Severity::BlockingError | Severity::NonblockingError => CSRSeverity::Error,
Severity::Warning => CSRSeverity::Warning,
Severity::Note => CSRSeverity::Note,
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2598,14 +2598,14 @@ fn exp(context: &mut Context, e: Box<E::Exp>) -> Box<N::Exp> {
}

EE::IfElse(eb, et, ef) => NE::IfElse(exp(context, eb), exp(context, et), exp(context, ef)),
EE::Match(esubject, sp!(_aloc, arms)) if arms.is_empty() => {
exp(context, esubject); // for error effect
let msg = "Invalid 'match' form. 'match' must have at least one arm";
context
.env
.add_diag(diag!(Syntax::InvalidMatch, (eloc, msg)));
NE::UnresolvedError
}
// EE::Match(esubject, sp!(_aloc, arms)) if arms.is_empty() => {
// exp(context, esubject); // for error effect
// let msg = "Invalid 'match' form. 'match' must have at least one arm";
// context
// .env
// .add_diag(diag!(Syntax::InvalidMatch, (eloc, msg)));
// NE::UnresolvedError
// }
EE::Match(esubject, sp!(aloc, arms)) => NE::Match(
exp(context, esubject),
sp(
Expand Down
176 changes: 173 additions & 3 deletions external-crates/move/crates/move-compiler/src/shared/ide.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
// Copyright (c) The Move Contributors
// SPDX-License-Identifier: Apache-2.0

use std::collections::BTreeSet;
use std::{collections::BTreeSet, fmt};

use crate::{
expansion::ast as E, naming::ast as N, parser::ast as P, shared::Name, typing::ast as T,
debug_display, diag, diagnostics::Diagnostic, expansion::ast as E, naming::ast as N,
parser::ast as P, shared::string_utils::format_oxford_list, shared::Name, typing::ast as T,
};

use move_ir_types::location::Loc;
Expand All @@ -16,7 +17,7 @@ use move_symbol_pool::Symbol;

#[derive(Debug, Clone, Default)]
pub struct IDEInfo {
annotations: Vec<(Loc, IDEAnnotation)>,
pub(crate) annotations: Vec<(Loc, IDEAnnotation)>,
}

#[derive(Debug, Clone)]
Expand All @@ -28,6 +29,8 @@ pub enum IDEAnnotation {
ExpandedLambda,
/// Autocomplete information.
AutocompleteInfo(Box<AutocompleteInfo>),
/// Match Missing Arm.
MissingMatchArms(Box<MissingMatchArmsInfo>),
}

#[derive(Debug, Clone)]
Expand All @@ -53,6 +56,54 @@ pub struct AutocompleteInfo {
pub fields: BTreeSet<Symbol>,
}

#[derive(Debug, Clone)]
pub struct MissingMatchArmsInfo {
/// A vector of arm patterns that can be inserted to make the match complete.
/// Note the span information on these is _wrong_ and must be recomputed after insertion.
pub arms: Vec<PatternSuggestion>,
}

/// Suggested new entries for a pattern. Note that any location information points to the
/// definition site. As this is largely suggested text, it lacks location information.
#[derive(Debug, Clone)]
pub enum PatternSuggestion {
Wildcard,
Binder(Symbol),
Value(E::Value_),
UnpackPositionalStruct {
module: E::ModuleIdent,
name: P::DatatypeName,
/// The number of wildcards to generate.
field_count: usize,
},
UnpackNamedStruct {
module: E::ModuleIdent,
name: P::DatatypeName,
/// The fields, in order, to generate
fields: Vec<Symbol>,
},
/// A tag-style variant that takes no arguments
UnpackEmptyVariant {
module: E::ModuleIdent,
enum_name: P::DatatypeName,
variant_name: P::VariantName,
},
UnpackPositionalVariant {
module: E::ModuleIdent,
enum_name: P::DatatypeName,
variant_name: P::VariantName,
/// The number of wildcards to generate.
field_count: usize,
},
UnpackNamedVariant {
module: E::ModuleIdent,
enum_name: P::DatatypeName,
variant_name: P::VariantName,
/// The fields, in order, to generate
fields: Vec<Symbol>,
},
}

//*************************************************************************************************
// Impls
//*************************************************************************************************
Expand Down Expand Up @@ -91,3 +142,122 @@ impl IntoIterator for IDEInfo {
self.annotations.into_iter()
}
}

impl From<(Loc, IDEAnnotation)> for Diagnostic {
fn from((loc, ann): (Loc, IDEAnnotation)) -> Self {
match ann {
IDEAnnotation::MacroCallInfo(info) => {
let MacroCallInfo {
module,
name,
method_name,
type_arguments,
by_value_args,
} = *info;
let mut diag = diag!(IDE::MacroCallInfo, (loc, "macro call info"));
diag.add_note(format!("Called {module}::{name}"));
if let Some(mname) = method_name {
diag.add_note(format!("as method call {mname}"));
}
if !type_arguments.is_empty() {
let tyargs_string = debug_display!(type_arguments).to_string();
diag.add_note(format!("Type arguments: {tyargs_string}"));
}
if let Some(entry) = by_value_args.first() {
let subject_arg_string = debug_display!(entry).to_string();
diag.add_note(format!("Subject arg: {subject_arg_string}"));
}
diag
}
IDEAnnotation::ExpandedLambda => {
diag!(IDE::ExpandedLambda, (loc, "expanded lambda"))
}
IDEAnnotation::AutocompleteInfo(info) => {
let AutocompleteInfo { methods, fields } = *info;
let names = methods
.into_iter()
.map(|(m, f)| format!("{m}::{f}"))
.chain(fields.into_iter().map(|n| format!("{n}")))
.collect::<Vec<_>>();
let msg = format!(
"Autocompletes to: {}",
format_oxford_list!("or", "'{}'", names)
);
diag!(IDE::Autocomplete, (loc, msg))
}
IDEAnnotation::MissingMatchArms(info) => {
let MissingMatchArmsInfo { arms } = *info;
let msg = format!("Missing arms: {}", format_oxford_list!("and", "'{}'", arms));
diag!(IDE::MissingMatchArms, (loc, msg))
}
}
}
}

impl fmt::Display for PatternSuggestion {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
use PatternSuggestion as PS;
match self {
PS::Wildcard => write!(f, "_"),
PS::Binder(n) => write!(f, "{n}"),
PS::Value(v) => write!(f, "{v}"),
PS::UnpackPositionalStruct {
module,
name,
field_count,
} => {
write!(f, "{module}::{name}")?;
let wildcards = std::iter::repeat("_")
.take(*field_count)
.collect::<Vec<_>>()
.join(", ");
write!(f, "({wildcards})")
}
PS::UnpackPositionalVariant {
module,
enum_name,
variant_name,
field_count,
} => {
write!(f, "{module}::{enum_name}::{variant_name}")?;
let wildcards = std::iter::repeat("_")
.take(*field_count)
.collect::<Vec<_>>()
.join(", ");
write!(f, "({wildcards})")
}
PS::UnpackNamedStruct {
module,
name,
fields,
} => {
write!(f, "{module}::{name} ")?;
let field_names = fields
.iter()
.map(|name| format!("{}", name))
.collect::<Vec<_>>()
.join(" , ");
write!(f, "{{ {field_names} }}")
}
PS::UnpackNamedVariant {
module,
enum_name,
variant_name,
fields,
} => {
write!(f, "{module}::{enum_name}::{variant_name} ")?;
let field_names = fields
.iter()
.map(|name| format!("{}", name))
.collect::<Vec<_>>()
.join(" , ");
write!(f, "{{ {field_names} }}")
}
PS::UnpackEmptyVariant {
module,
enum_name,
variant_name,
} => write!(f, "{module}::{enum_name}::{variant_name}"),
}
}
}
29 changes: 28 additions & 1 deletion external-crates/move/crates/move-compiler/src/shared/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -665,10 +665,20 @@ impl CompilationEnv {
}

pub fn extend_ide_info(&mut self, info: IDEInfo) {
if self.flags().ide_test_mode() {
for entry in info.annotations.iter() {
let diag = entry.clone().into();
self.diags.add(diag);
}
}
self.ide_information.extend(info);
}

pub fn add_ide_annotation(&mut self, loc: Loc, info: IDEAnnotation) {
if self.flags().ide_test_mode() {
let diag = (loc, info.clone()).into();
self.diags.add(diag);
}
self.ide_information.add_ide_annotation(loc, info);
}
}
Expand Down Expand Up @@ -763,7 +773,11 @@ pub struct Flags {
#[clap(skip)]
keep_testing_functions: bool,

/// If set, all warnings are silenced
/// If set, we are in IDE testing mode. This will report IDE annotations as diagnostics.
#[clap(skip = false)]
ide_test_mode: bool,

/// If set, we are in IDE mode.
#[clap(skip = false)]
ide_mode: bool,
}
Expand All @@ -779,6 +793,7 @@ impl Flags {
json_errors: false,
keep_testing_functions: false,
ide_mode: false,
ide_test_mode: false,
}
}

Expand All @@ -792,6 +807,7 @@ impl Flags {
silence_warnings: false,
keep_testing_functions: false,
ide_mode: false,
ide_test_mode: false,
}
}

Expand Down Expand Up @@ -830,6 +846,13 @@ impl Flags {
}
}

pub fn set_ide_test_mode(self, value: bool) -> Self {
Self {
ide_test_mode: value,
..self
}
}

pub fn set_ide_mode(self, value: bool) -> Self {
Self {
ide_mode: value,
Expand Down Expand Up @@ -869,6 +892,10 @@ impl Flags {
self.silence_warnings
}

pub fn ide_test_mode(&self) -> bool {
self.ide_test_mode
}

pub fn ide_mode(&self) -> bool {
self.ide_mode
}
Expand Down
18 changes: 18 additions & 0 deletions external-crates/move/crates/move-compiler/src/typing/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +789,24 @@ impl<'env> Context<'env> {
}
}

/// Indicates if the enum variant is empty.
pub fn enum_variant_is_empty(
&self,
module: &ModuleIdent,
enum_name: &DatatypeName,
variant_name: &VariantName,
) -> bool {
let vdef = self
.enum_definition(module, enum_name)
.variants
.get(variant_name)
.expect("ICE should have failed during naming");
match &vdef.fields {
N::VariantFields::Empty => true,
N::VariantFields::Defined(_, _m) => false,
}
}

/// Indicates if the enum variant is positional. Returns false on empty or missing.
pub fn enum_variant_is_positional(
&self,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -525,5 +525,6 @@ pub fn ide_annotation(context: &mut Context, annotation: &mut IDEAnnotation) {
}
IDEAnnotation::ExpandedLambda => (),
IDEAnnotation::AutocompleteInfo(_) => (),
IDEAnnotation::MissingMatchArms(_) => (),
}
}
Loading

0 comments on commit 1f147ed

Please sign in to comment.