Skip to content

Commit

Permalink
[CLI] Add compiler-message-format-json experiment value to `aptos m…
Browse files Browse the repository at this point in the history
…ove compile` and `aptos move lint` (#15540)
  • Loading branch information
mkurnikov authored Dec 23, 2024
1 parent 8a0f983 commit f5d0131
Show file tree
Hide file tree
Showing 18 changed files with 253 additions and 46 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions crates/aptos/src/move_tool/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ pub struct LintPackage {
/// See <https://github.com/aptos-labs/aptos-core/issues/10335>
#[clap(long, env = "APTOS_CHECK_TEST_CODE")]
pub check_test_code: bool,

/// Experiments
#[clap(long, hide(true))]
pub experiments: Vec<String>,
}

impl LintPackage {
Expand All @@ -89,6 +93,7 @@ impl LintPackage {
language_version,
skip_attribute_checks,
check_test_code,
experiments,
} = self.clone();
MovePackageDir {
dev,
Expand All @@ -100,6 +105,7 @@ impl LintPackage {
language_version,
skip_attribute_checks,
check_test_code,
experiments,
..MovePackageDir::new()
}
}
Expand Down
2 changes: 2 additions & 0 deletions third_party/move/move-compiler-v2/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ abstract-domain-derive = { path = "../move-model/bytecode/abstract_domain_derive
anyhow = { workspace = true }
bcs = { workspace = true }
clap = { workspace = true, features = ["derive", "env"] }
codespan = { workspace = true }
codespan-reporting = { workspace = true, features = ["serde", "serialization"] }
ethnum = { workspace = true }
flexi_logger = { workspace = true }
Expand All @@ -35,6 +36,7 @@ move-symbol-pool = { workspace = true }
num = { workspace = true }
once_cell = { workspace = true }
petgraph = { workspace = true }
serde_json = { workspace = true }

[dev-dependencies]
anyhow = { workspace = true }
Expand Down
33 changes: 33 additions & 0 deletions third_party/move/move-compiler-v2/src/diagnostics/human.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Copyright © Aptos Foundation
// SPDX-License-Identifier: Apache-2.0

use crate::diagnostics::Emitter;
use codespan::{FileId, Files};
use codespan_reporting::{
diagnostic::Diagnostic,
term::{emit, termcolor::WriteColor, Config},
};

/// It's used in the native aptos-cli output to show error messages.
/// Wraps the `codespan_reporting::term::emit()` method.
pub struct HumanEmitter<'w, W: WriteColor> {
writer: &'w mut W,
}

impl<'w, W> HumanEmitter<'w, W>
where
W: WriteColor,
{
pub fn new(writer: &'w mut W) -> Self {
HumanEmitter { writer }
}
}

impl<'w, W> Emitter for HumanEmitter<'w, W>
where
W: WriteColor,
{
fn emit(&mut self, source_files: &Files<String>, diag: &Diagnostic<FileId>) {
emit(&mut self.writer, &Config::default(), source_files, diag).expect("emit must not fail")
}
}
44 changes: 44 additions & 0 deletions third_party/move/move-compiler-v2/src/diagnostics/json.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Copyright © Aptos Foundation
// SPDX-License-Identifier: Apache-2.0

use crate::diagnostics::Emitter;
use codespan::{FileId, Files};
use codespan_reporting::diagnostic::{Diagnostic, Label};
use std::io::Write;

/// Shows compiler errors as a structured JSON output.
/// Exists to support various tools external to the aptos-cli, i.e. IDEs.
pub struct JsonEmitter<'w, W: Write> {
writer: &'w mut W,
}

impl<'w, W: Write> JsonEmitter<'w, W> {
pub fn new(writer: &'w mut W) -> Self {
JsonEmitter { writer }
}
}

impl<'w, W: Write> Emitter for JsonEmitter<'w, W> {
fn emit(&mut self, source_files: &Files<String>, diag: &Diagnostic<FileId>) {
let fpath_labels = diag
.labels
.iter()
.map(|label| {
let fpath = codespan_reporting::files::Files::name(source_files, label.file_id)
.expect("always Ok() in the impl")
.to_string();
Label::new(label.style, fpath, label.range.clone())
})
.collect();
let mut json_diag = Diagnostic::new(diag.severity)
.with_message(diag.message.clone())
.with_labels(fpath_labels)
.with_notes(diag.notes.clone());
if let Some(code) = &diag.code {
json_diag = json_diag.with_code(code)
}
serde_json::to_writer(&mut self.writer, &json_diag).expect("it should be serializable");
writeln!(&mut self.writer)
.expect("dest is stderr / in-memory buffer, it should always be available");
}
}
58 changes: 58 additions & 0 deletions third_party/move/move-compiler-v2/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// Copyright © Aptos Foundation
// SPDX-License-Identifier: Apache-2.0

use crate::{
diagnostics::{human::HumanEmitter, json::JsonEmitter},
options, Experiment,
};
use anyhow::bail;
use codespan::{FileId, Files};
use codespan_reporting::{
diagnostic::{Diagnostic, Severity},
term::termcolor::WriteColor,
};
use move_model::model::GlobalEnv;

pub mod human;
pub mod json;

impl options::Options {
pub fn error_emitter<'w, W>(&self, dest: &'w mut W) -> Box<dyn Emitter + 'w>
where
W: WriteColor,
{
if self.experiment_on(Experiment::MESSAGE_FORMAT_JSON) {
Box::new(JsonEmitter::new(dest))
} else {
Box::new(HumanEmitter::new(dest))
}
}
}

pub trait Emitter {
fn emit(&mut self, source_files: &Files<String>, diag: &Diagnostic<FileId>);

/// Writes accumulated diagnostics of given or higher severity.
fn report_diag(&mut self, global_env: &GlobalEnv, severity: Severity) {
global_env.report_diag_with_filter(
|files, diag| self.emit(files, diag),
|d| d.severity >= severity,
);
}

/// Helper function to report diagnostics, check for errors, and fail with a message on
/// errors. This function is idempotent and will not report the same diagnostics again.
fn check_diag(
&mut self,
global_env: &GlobalEnv,
report_severity: Severity,
msg: &str,
) -> anyhow::Result<()> {
self.report_diag(global_env, report_severity);
if global_env.has_errors() {
bail!("exiting with {}", msg);
} else {
Ok(())
}
}
}
6 changes: 6 additions & 0 deletions third_party/move/move-compiler-v2/src/experiments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,11 @@ pub static EXPERIMENTS: Lazy<BTreeMap<String, Experiment>> = Lazy::new(|| {
.to_string(),
default: Given(false),
},
Experiment {
name: Experiment::MESSAGE_FORMAT_JSON.to_string(),
description: "Enable json format for compiler messages".to_string(),
default: Given(false),
},
];
experiments
.into_iter()
Expand Down Expand Up @@ -302,6 +307,7 @@ impl Experiment {
pub const LAMBDA_LIFTING: &'static str = "lambda-lifting";
pub const LAMBDA_VALUES: &'static str = "lambda-values";
pub const LINT_CHECKS: &'static str = "lint-checks";
pub const MESSAGE_FORMAT_JSON: &'static str = "compiler-message-format-json";
pub const OPTIMIZE: &'static str = "optimize";
pub const OPTIMIZE_EXTRA: &'static str = "optimize-extra";
pub const OPTIMIZE_WAITING_FOR_COMPARE_TESTS: &'static str =
Expand Down
38 changes: 22 additions & 16 deletions third_party/move/move-compiler-v2/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// SPDX-License-Identifier: Apache-2.0

mod bytecode_generator;
pub mod diagnostics;
pub mod env_pipeline;
mod experiments;
pub mod external_checks;
Expand All @@ -14,6 +15,7 @@ pub mod pipeline;
pub mod plan_builder;

use crate::{
diagnostics::Emitter,
env_pipeline::{
acquires_checker, ast_simplifier, cyclic_instantiation_checker, flow_insensitive_checkers,
function_checker, inliner, lambda_lifter, lambda_lifter::LambdaLiftingOptions,
Expand Down Expand Up @@ -71,38 +73,40 @@ use move_stackless_bytecode::function_target_pipeline::{
};
use move_symbol_pool::Symbol;
pub use options::Options;
use std::{collections::BTreeSet, io::Write, path::Path};
use std::{collections::BTreeSet, path::Path};

/// Run Move compiler and print errors to stderr.
pub fn run_move_compiler_to_stderr(
options: Options,
) -> anyhow::Result<(GlobalEnv, Vec<AnnotatedCompiledUnit>)> {
let mut error_writer = StandardStream::stderr(ColorChoice::Auto);
run_move_compiler(&mut error_writer, options)
let mut stderr = StandardStream::stderr(ColorChoice::Auto);
let mut emitter = options.error_emitter(&mut stderr);
run_move_compiler(emitter.as_mut(), options)
}

/// Run move compiler and print errors to given writer. Returns the set of compiled units.
pub fn run_move_compiler<W>(
error_writer: &mut W,
pub fn run_move_compiler<E>(
emitter: &mut E,
options: Options,
) -> anyhow::Result<(GlobalEnv, Vec<AnnotatedCompiledUnit>)>
where
W: WriteColor + Write,
E: Emitter + ?Sized,
{
logging::setup_logging();
info!("Move Compiler v2");

// Run context check.
let mut env = run_checker_and_rewriters(options.clone())?;
check_errors(&env, error_writer, "checking errors")?;
check_errors(&env, emitter, "checking errors")?;

if options.experiment_on(Experiment::STOP_BEFORE_STACKLESS_BYTECODE) {
std::process::exit(0)
}

// Run code generator
let mut targets = run_bytecode_gen(&env);
check_errors(&env, error_writer, "code generation errors")?;
check_errors(&env, emitter, "code generation errors")?;

debug!("After bytecode_gen, GlobalEnv={}", env.dump_env());

// Run transformation pipeline
Expand All @@ -129,14 +133,14 @@ where
} else {
pipeline.run_with_hook(&env, &mut targets, |_| {}, |_, _, _| !env.has_errors())
}
check_errors(&env, error_writer, "stackless-bytecode analysis errors")?;
check_errors(&env, emitter, "stackless-bytecode analysis errors")?;

if options.experiment_on(Experiment::STOP_BEFORE_FILE_FORMAT) {
std::process::exit(0)
}

let modules_and_scripts = run_file_format_gen(&mut env, &targets);
check_errors(&env, error_writer, "assembling errors")?;
check_errors(&env, emitter, "assembling errors")?;

debug!(
"File format bytecode:\n{}",
Expand All @@ -145,7 +149,7 @@ where

let annotated_units = annotate_units(modules_and_scripts);
run_bytecode_verifier(&annotated_units, &mut env);
check_errors(&env, error_writer, "bytecode verification errors")?;
check_errors(&env, emitter, "bytecode verification errors")?;

// Finally mark this model to be generated by v2
env.set_compiler_v2(true);
Expand All @@ -163,7 +167,8 @@ pub fn run_move_compiler_for_analysis(
options.whole_program = true; // will set `treat_everything_as_target`
options = options.set_experiment(Experiment::SPEC_REWRITE, true);
options = options.set_experiment(Experiment::ATTACH_COMPILED_MODULE, true);
let (env, _units) = run_move_compiler(error_writer, options)?;
let mut emitter = options.error_emitter(error_writer);
let (env, _units) = run_move_compiler(emitter.as_mut(), options)?;
// Reset for subsequent analysis
env.treat_everything_as_target(false);
Ok(env)
Expand Down Expand Up @@ -605,13 +610,14 @@ fn get_vm_error_loc(env: &GlobalEnv, source_map: &SourceMap, e: &VMError) -> Opt
}

/// Report any diags in the env to the writer and fail if there are errors.
pub fn check_errors<W>(env: &GlobalEnv, error_writer: &mut W, msg: &str) -> anyhow::Result<()>
pub fn check_errors<E>(env: &GlobalEnv, emitter: &mut E, msg: &str) -> anyhow::Result<()>
where
W: WriteColor + Write,
E: Emitter + ?Sized,
{
let options = env.get_extension::<Options>().unwrap_or_default();
env.report_diag(error_writer, options.report_severity());
env.check_diag(error_writer, options.report_severity(), msg)

emitter.report_diag(env, options.report_severity());
emitter.check_diag(env, options.report_severity(), msg)
}

/// Annotate the given compiled units.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

Diagnostics:
{"severity":"Error","code":null,"message":"cannot use `bool` with an operator which expects a value of type `integer`","labels":[{"style":"Primary","file_id":"tests/compiler-message-format-json/errors.move","range":{"start":51,"end":55},"message":""}],"notes":[]}
{"severity":"Error","code":null,"message":"cannot use `bool` with an operator which expects a value of type `integer`","labels":[{"style":"Primary","file_id":"tests/compiler-message-format-json/errors.move","range":{"start":69,"end":73},"message":""}],"notes":[]}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module 0x42::errors {
fun main() {
1 + true;
2 + true;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

Diagnostics:
{"severity":"Warning","code":null,"message":"Unused local variable `a`. Consider removing or prefixing with an underscore: `_a`","labels":[{"style":"Primary","file_id":"tests/compiler-message-format-json/warnings.move","range":{"start":53,"end":54},"message":""}],"notes":[]}
{"severity":"Warning","code":null,"message":"Unused local variable `b`. Consider removing or prefixing with an underscore: `_b`","labels":[{"style":"Primary","file_id":"tests/compiler-message-format-json/warnings.move","range":{"start":72,"end":73},"message":""}],"notes":[]}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module 0x42::warnings {
fun main() {
let a = 1;
let b = 2;
}
}
Loading

0 comments on commit f5d0131

Please sign in to comment.