Skip to content

Commit

Permalink
[move] Update external workflow to check formatting for _all_ packages (
Browse files Browse the repository at this point in the history
MystenLabs#18738)

## Description 

This updates the external workflow to check all crates. It appears that,
without this, `cargo` will not check crates with `publish = false` by
default.

## Test plan 

This updates tests to ensure formatting is applied uniformly in the
external crates, plus a few commits explicitly tested behavior.

---

## 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:
- [ ] REST API:
  • Loading branch information
cgswords authored Jul 19, 2024
1 parent 77bae85 commit 090bec4
Show file tree
Hide file tree
Showing 20 changed files with 26 additions and 75 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/external.yml
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ jobs:
uses: actions-rs/cargo@ae10961054e4aa8b4aa7dffede299aaf087aa33b # [email protected]
with:
command: fmt
args: --check
args: --check --all

cargo-deny:
name: cargo-deny (advisories, licenses, bans, ...)
Expand Down
6 changes: 1 addition & 5 deletions external-crates/move/crates/move-core-types/src/effects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,7 @@
// Copyright (c) The Move Contributors
// SPDX-License-Identifier: Apache-2.0

use crate::{
account_address::AccountAddress,
identifier::Identifier,
language_storage::ModuleId,
};
use crate::{account_address::AccountAddress, identifier::Identifier, language_storage::ModuleId};
use anyhow::{bail, Result};
use std::collections::btree_map::{self, BTreeMap};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::{
use itertools::Itertools;
use move_binary_format::file_format::CodeOffset;
use move_model::{
model::{FunId, FunctionEnv, FunctionVisibility, GlobalEnv, Loc, ModuleEnv, DatatypeId},
model::{DatatypeId, FunId, FunctionEnv, FunctionVisibility, GlobalEnv, Loc, ModuleEnv},
symbol::{Symbol, SymbolPool},
ty::{Type, TypeDisplayContext},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

use move_model::{
ast::TempIndex,
model::{FieldId, FunId, FunctionEnv, ModuleId, NodeId, StructEnv, DatatypeId},
model::{DatatypeId, FieldId, FunId, FunctionEnv, ModuleId, NodeId, StructEnv},
ty::Type,
};
use std::collections::BTreeMap;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,7 @@ impl<S: MoveResolver> TransactionDataCache<S> {

if !modules.is_empty() {
change_set
.add_account_changeset(
addr,
AccountChangeSet::from_modules(modules),
)
.add_account_changeset(addr, AccountChangeSet::from_modules(modules))
.expect("accounts should be unique");
}
}
Expand Down
5 changes: 1 addition & 4 deletions external-crates/move/crates/move-vm-runtime/src/move_vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,7 @@ impl MoveVM {
) -> VMResult<Arc<CompiledModule>> {
self.runtime
.loader()
.load_module(
module_id,
&TransactionDataCache::new(remote),
)
.load_module(module_id, &TransactionDataCache::new(remote))
.map(|(compiled, _)| compiled)
}

Expand Down
7 changes: 1 addition & 6 deletions external-crates/move/crates/move-vm-runtime/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,12 +179,7 @@ impl<'r, 'l, S: MoveResolver> Session<'r, 'l, S> {
}

/// Same like `finish`, but also extracts the native context extensions from the session.
pub fn finish_with_extensions(
self,
) -> (
VMResult<(ChangeSet, NativeContextExtensions<'r>)>,
S,
) {
pub fn finish_with_extensions(self) -> (VMResult<(ChangeSet, NativeContextExtensions<'r>)>, S) {
let Session {
data_cache,
native_extensions,
Expand Down
3 changes: 1 addition & 2 deletions external-crates/move/crates/move-vm-types/src/data_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@

use move_binary_format::errors::{PartialVMResult, VMResult};
use move_core_types::{
account_address::AccountAddress, identifier::IdentStr,
language_storage::ModuleId,
account_address::AccountAddress, identifier::IdentStr, language_storage::ModuleId,
};

/// Provide an implementation for bytecodes related to data with a given data store.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
use move_binary_format::{
errors::{verification_error, Location, PartialVMResult, VMResult},
file_format::{
CompiledModule, Constant, FunctionHandle, FunctionHandleIndex, FunctionInstantiation,
ModuleHandle, Signature, StructFieldInformation, DatatypeHandle, DatatypeHandleIndex,
TableIndex,
CompiledModule, Constant, DatatypeHandle, DatatypeHandleIndex, FunctionHandle,
FunctionHandleIndex, FunctionInstantiation, ModuleHandle, Signature,
StructFieldInformation, TableIndex,
},
IndexKind,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,7 @@ impl<S: MoveResolver> TransactionDataCache<S> {
}
if !modules.is_empty() {
change_set
.add_account_changeset(
addr,
AccountChangeSet::from_modules(modules),
)
.add_account_changeset(addr, AccountChangeSet::from_modules(modules))
.expect("accounts should be unique");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,7 @@ impl MoveVM {
) -> VMResult<Arc<CompiledModule>> {
self.runtime
.loader()
.load_module(
module_id,
&TransactionDataCache::new(remote),
)
.load_module(module_id, &TransactionDataCache::new(remote))
.map(|(compiled, _)| compiled)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,12 +165,7 @@ impl<'r, 'l, S: MoveResolver> Session<'r, 'l, S> {
}

/// Same like `finish`, but also extracts the native context extensions from the session.
pub fn finish_with_extensions(
self,
) -> (
VMResult<(ChangeSet, NativeContextExtensions<'r>)>,
S,
) {
pub fn finish_with_extensions(self) -> (VMResult<(ChangeSet, NativeContextExtensions<'r>)>, S) {
let Session {
data_cache,
native_extensions,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
use move_binary_format::{
errors::{verification_error, Location, PartialVMResult, VMResult},
file_format::{
CompiledModule, Constant, FunctionHandle, FunctionHandleIndex, FunctionInstantiation,
ModuleHandle, Signature, StructFieldInformation, DatatypeHandle, DatatypeHandleIndex,
TableIndex,
CompiledModule, Constant, DatatypeHandle, DatatypeHandleIndex, FunctionHandle,
FunctionHandleIndex, FunctionInstantiation, ModuleHandle, Signature,
StructFieldInformation, TableIndex,
},
IndexKind,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,7 @@ impl<S: MoveResolver> TransactionDataCache<S> {
}
if !modules.is_empty() {
change_set
.add_account_changeset(
addr,
AccountChangeSet::from_modules(modules),
)
.add_account_changeset(addr, AccountChangeSet::from_modules(modules))
.expect("accounts should be unique");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,7 @@ impl MoveVM {
) -> VMResult<Arc<CompiledModule>> {
self.runtime
.loader()
.load_module(
module_id,
&TransactionDataCache::new(remote),
)
.load_module(module_id, &TransactionDataCache::new(remote))
.map(|(compiled, _)| compiled)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use move_vm_config::runtime::VMConfig;
use move_vm_types::{
data_store::DataStore,
gas::GasMeter,
loaded_data::runtime_types::{CachedTypeIndex, CachedDatatype, Type},
loaded_data::runtime_types::{CachedDatatype, CachedTypeIndex, Type},
values::{Locals, Reference, VMValueCast, Value},
};
use std::{borrow::Borrow, collections::BTreeSet, sync::Arc};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use move_core_types::{
use move_vm_types::{
data_store::DataStore,
gas::GasMeter,
loaded_data::runtime_types::{CachedTypeIndex, CachedDatatype, Type},
loaded_data::runtime_types::{CachedDatatype, CachedTypeIndex, Type},
};
use std::{borrow::Borrow, sync::Arc};

Expand Down Expand Up @@ -179,12 +179,7 @@ impl<'r, 'l, S: MoveResolver> Session<'r, 'l, S> {
}

/// Same like `finish`, but also extracts the native context extensions from the session.
pub fn finish_with_extensions(
self,
) -> (
VMResult<(ChangeSet, NativeContextExtensions<'r>)>,
S,
) {
pub fn finish_with_extensions(self) -> (VMResult<(ChangeSet, NativeContextExtensions<'r>)>, S) {
let Session {
data_cache,
native_extensions,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,7 @@ impl<S: MoveResolver> TransactionDataCache<S> {
}
if !modules.is_empty() {
change_set
.add_account_changeset(
addr,
AccountChangeSet::from_modules(modules),
)
.add_account_changeset(addr, AccountChangeSet::from_modules(modules))
.expect("accounts should be unique");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,7 @@ impl MoveVM {
) -> VMResult<Arc<CompiledModule>> {
self.runtime
.loader()
.load_module(
module_id,
&TransactionDataCache::new(remote),
)
.load_module(module_id, &TransactionDataCache::new(remote))
.map(|(compiled, _)| compiled)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use move_core_types::{
use move_vm_types::{
data_store::DataStore,
gas::GasMeter,
loaded_data::runtime_types::{CachedTypeIndex, CachedDatatype, Type},
loaded_data::runtime_types::{CachedDatatype, CachedTypeIndex, Type},
};
use std::{borrow::Borrow, sync::Arc};

Expand Down Expand Up @@ -179,12 +179,7 @@ impl<'r, 'l, S: MoveResolver> Session<'r, 'l, S> {
}

/// Same like `finish`, but also extracts the native context extensions from the session.
pub fn finish_with_extensions(
self,
) -> (
VMResult<(ChangeSet, NativeContextExtensions<'r>)>,
S,
) {
pub fn finish_with_extensions(self) -> (VMResult<(ChangeSet, NativeContextExtensions<'r>)>, S) {
let Session {
data_cache,
native_extensions,
Expand Down

0 comments on commit 090bec4

Please sign in to comment.