extern/nushell
1
0
Fork 1
mirror of https://github.com/nushell/nushell.git synced 2024-12-23 15:39:06 +01:00
Code Issues Packages Projects Releases Wiki Activity

Remove unused Index(Mut) impls on AST types (#11903)

Browse Source 
# Description
Both `Block` and `Pipeline` had `Index`/`IndexMut` implementations to
access their elements, that are currently unused.
Explicit helpers or iteration would generally be preferred anyways but
in the current state the inner containers are `pub` and are liberally
used. (Sometimes with potentially panicking indexing or also iteration)

As it is potentially unclear what the meaning of the element from a
block or pipeline queried by a usize is, let's remove it entirely until
we come up with a better API.

# User-Facing Changes
None

Plugin authors shouldn't dig into AST internals
This commit is contained in:
Stefan Holderbach 2024-02-21 11:02:30 +01:00 committed by GitHub
parent b23fe30530
commit 6e590fe0a2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 66 additions and 96 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

128
crates/nu-parser/tests/test_parser.rs
View File

@ -73,7 +73,7 @@ fn test_int(
} else {
assert!(err.is_none(), "{test_tag}: unexpected error {err:#?}");
assert_eq!(block.len(), 1, "{test_tag}: result block length > 1");
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(
expressions.len(),
1,
@ -84,7 +84,7 @@ fn test_int(
Expression {
expr: observed_val, ..
},
) = &expressions[0]
) = &expressions.elements[0]
{
compare_rhs_binaryOp(test_tag, &expected_val, observed_val);
}
@ -287,10 +287,10 @@ pub fn parse_int() {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 1);
assert!(matches!(
expressions[0],
expressions.elements[0],
PipelineElement::Expression(
_,
Expression {
@ -310,10 +310,10 @@ pub fn parse_int_with_underscores() {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 1);
assert!(matches!(
expressions[0],
expressions.elements[0],
PipelineElement::Expression(
_,
Expression {
@ -340,11 +340,11 @@ pub fn parse_cell_path() {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 1);
// hoo boy this pattern matching is a pain
if let PipelineElement::Expression(_, expr) = &expressions[0] {
if let PipelineElement::Expression(_, expr) = &expressions.elements[0] {
if let Expr::FullCellPath(b) = &expr.expr {
assert!(matches!(
b.head,
@ -395,11 +395,11 @@ pub fn parse_cell_path_optional() {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 1);
// hoo boy this pattern matching is a pain
if let PipelineElement::Expression(_, expr) = &expressions[0] {
if let PipelineElement::Expression(_, expr) = &expressions.elements[0] {
if let Expr::FullCellPath(b) = &expr.expr {
assert!(matches!(
b.head,
@ -442,9 +442,9 @@ pub fn parse_binary_with_hex_format() {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 1);
if let PipelineElement::Expression(_, expr) = &expressions[0] {
if let PipelineElement::Expression(_, expr) = &expressions.elements[0] {
assert_eq!(expr.expr, Expr::Binary(vec![0x13]))
} else {
panic!("Not an expression")
@ -460,9 +460,9 @@ pub fn parse_binary_with_incomplete_hex_format() {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 1);
if let PipelineElement::Expression(_, expr) = &expressions[0] {
if let PipelineElement::Expression(_, expr) = &expressions.elements[0] {
assert_eq!(expr.expr, Expr::Binary(vec![0x03]))
} else {
panic!("Not an expression")
@ -478,9 +478,9 @@ pub fn parse_binary_with_binary_format() {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 1);
if let PipelineElement::Expression(_, expr) = &expressions[0] {
if let PipelineElement::Expression(_, expr) = &expressions.elements[0] {
assert_eq!(expr.expr, Expr::Binary(vec![0b10101000]))
} else {
panic!("Not an expression")
@ -496,9 +496,9 @@ pub fn parse_binary_with_incomplete_binary_format() {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 1);
if let PipelineElement::Expression(_, expr) = &expressions[0] {
if let PipelineElement::Expression(_, expr) = &expressions.elements[0] {
assert_eq!(expr.expr, Expr::Binary(vec![0b00000010]))
} else {
panic!("Not an expression")
@ -514,9 +514,9 @@ pub fn parse_binary_with_octal_format() {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 1);
if let PipelineElement::Expression(_, expr) = &expressions[0] {
if let PipelineElement::Expression(_, expr) = &expressions.elements[0] {
assert_eq!(expr.expr, Expr::Binary(vec![0o250]))
} else {
panic!("Not an expression")
@ -532,9 +532,9 @@ pub fn parse_binary_with_incomplete_octal_format() {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 1);
if let PipelineElement::Expression(_, expr) = &expressions[0] {
if let PipelineElement::Expression(_, expr) = &expressions.elements[0] {
assert_eq!(expr.expr, Expr::Binary(vec![0o2]))
} else {
panic!("Not an expression")
@ -550,9 +550,9 @@ pub fn parse_binary_with_invalid_octal_format() {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 1);
if let PipelineElement::Expression(_, expr) = &expressions[0] {
if let PipelineElement::Expression(_, expr) = &expressions.elements[0] {
assert!(!matches!(&expr.expr, Expr::Binary(_)))
} else {
panic!("Not an expression")
@ -570,9 +570,9 @@ pub fn parse_binary_with_multi_byte_char() {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 1);
if let PipelineElement::Expression(_, expr) = &expressions[0] {
if let PipelineElement::Expression(_, expr) = &expressions.elements[0] {
assert!(!matches!(&expr.expr, Expr::Binary(_)))
} else {
panic!("Not an expression")
@ -592,7 +592,7 @@ pub fn parse_call() {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 1);
if let PipelineElement::Expression(
@ -601,7 +601,7 @@ pub fn parse_call() {
expr: Expr::Call(call),
..
},
) = &expressions[0]
) = &expressions.elements[0]
{
assert_eq!(call.decl_id, 0);
}
@ -651,7 +651,7 @@ pub fn parse_call_short_flag_batch_arg_allowed() {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 1);
if let PipelineElement::Expression(
@ -660,7 +660,7 @@ pub fn parse_call_short_flag_batch_arg_allowed() {
expr: Expr::Call(call),
..
},
) = &expressions[0]
) = &expressions.elements[0]
{
assert_eq!(call.decl_id, 0);
assert_eq!(call.arguments.len(), 2);
@ -768,10 +768,10 @@ fn test_nothing_comparison_eq() {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 1);
assert!(matches!(
&expressions[0],
&expressions.elements[0],
PipelineElement::Expression(
_,
Expression {
@ -815,10 +815,10 @@ fn test_nothing_comparison_neq() {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 1);
assert!(matches!(
&expressions[0],
&expressions.elements[0],
PipelineElement::Expression(
_,
Expression {
@ -841,9 +841,9 @@ mod string {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 1);
if let PipelineElement::Expression(_, expr) = &expressions[0] {
if let PipelineElement::Expression(_, expr) = &expressions.elements[0] {
assert_eq!(expr.expr, Expr::String("hello nushell".to_string()))
} else {
panic!("Not an expression")
@ -865,10 +865,10 @@ mod string {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 1);
if let PipelineElement::Expression(_, expr) = &expressions[0] {
if let PipelineElement::Expression(_, expr) = &expressions.elements[0] {
let subexprs: Vec<&Expr> = match expr {
Expression {
expr: Expr::StringInterpolation(expressions),
@ -897,11 +897,11 @@ mod string {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 1);
if let PipelineElement::Expression(_, expr) = &expressions[0] {
if let PipelineElement::Expression(_, expr) = &expressions.elements[0] {
let subexprs: Vec<&Expr> = match expr {
Expression {
expr: Expr::StringInterpolation(expressions),
@ -928,11 +928,11 @@ mod string {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 1);
if let PipelineElement::Expression(_, expr) = &expressions[0] {
if let PipelineElement::Expression(_, expr) = &expressions.elements[0] {
let subexprs: Vec<&Expr> = match expr {
Expression {
expr: Expr::StringInterpolation(expressions),
@ -961,11 +961,11 @@ mod string {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 1);
if let PipelineElement::Expression(_, expr) = &expressions[0] {
if let PipelineElement::Expression(_, expr) = &expressions.elements[0] {
let subexprs: Vec<&Expr> = match expr {
Expression {
expr: Expr::StringInterpolation(expressions),
@ -1085,7 +1085,7 @@ mod range {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1, "{tag}: block length");
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 1, "{tag}: expression length");
if let PipelineElement::Expression(
_,
@ -1102,7 +1102,7 @@ mod range {
),
..
},
) = expressions[0]
) = expressions.elements[0]
{
assert_eq!(
the_inclusion, inclusion,
@ -1144,7 +1144,7 @@ mod range {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 2, "{tag} block len 2");
let expressions = &block[1];
let expressions = &block.pipelines[1];
assert_eq!(expressions.len(), 1, "{tag}: expression length 1");
if let PipelineElement::Expression(
_,
@ -1161,7 +1161,7 @@ mod range {
),
..
},
) = expressions[0]
) = expressions.elements[0]
{
assert_eq!(
the_inclusion, inclusion,
@ -1190,7 +1190,7 @@ mod range {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1, "{tag}: block len 1");
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 1, "{tag}: expression length 1");
if let PipelineElement::Expression(
_,
@ -1207,7 +1207,7 @@ mod range {
),
..
},
) = expressions[0]
) = expressions.elements[0]
{
assert_eq!(
the_inclusion, inclusion,
@ -1236,7 +1236,7 @@ mod range {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1, "{tag}: block len 1");
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 1, "{tag}: expression length 1");
if let PipelineElement::Expression(
_,
@ -1253,7 +1253,7 @@ mod range {
),
..
},
) = expressions[0]
) = expressions.elements[0]
{
assert_eq!(
the_inclusion, inclusion,
@ -1282,7 +1282,7 @@ mod range {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1, "{tag}: block length 1");
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 1, "{tag}: expression length 1");
if let PipelineElement::Expression(
_,
@ -1299,7 +1299,7 @@ mod range {
),
..
},
) = expressions[0]
) = expressions.elements[0]
{
assert_eq!(
the_inclusion, inclusion,
@ -1672,10 +1672,10 @@ mod input_types {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 2);
match &expressions[0] {
match &expressions.elements[0] {
PipelineElement::Expression(
_,
Expression {
@ -1689,7 +1689,7 @@ mod input_types {
_ => panic!("Expected expression Call not found"),
}
match &expressions[1] {
match &expressions.elements[1] {
PipelineElement::Expression(
_,
Expression {
@ -1719,8 +1719,8 @@ mod input_types {
engine_state.merge_delta(delta).unwrap();
let expressions = &block[0];
match &expressions[3] {
let expressions = &block.pipelines[0];
match &expressions.elements[3] {
PipelineElement::Expression(
_,
Expression {
@ -1735,10 +1735,10 @@ mod input_types {
Expr::Subexpression(id) => {
let block = engine_state.get_block(*id);
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 2);
match &expressions[1] {
match &expressions.elements[1] {
PipelineElement::Expression(
_,
Expression {
@ -1777,8 +1777,8 @@ mod input_types {
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
match &expressions[2] {
let expressions = &block.pipelines[0];
match &expressions.elements[2] {
PipelineElement::Expression(
_,
Expression {
@ -1792,7 +1792,7 @@ mod input_types {
_ => panic!("Expected expression Call not found"),
}
match &expressions[3] {
match &expressions.elements[3] {
PipelineElement::Expression(
_,
Expression {

4
crates/nu-parser/tests/test_parser_unicode_escapes.rs
View File

@ -19,9 +19,9 @@ pub fn do_test(test: &[u8], expected: &str, error_contains: Option<&str>) {
match working_set.parse_errors.first() {
None => {
assert_eq!(block.len(), 1);
let expressions = &block[0];
let expressions = &block.pipelines[0];
assert_eq!(expressions.len(), 1);
if let PipelineElement::Expression(_, expr) = &expressions[0] {
if let PipelineElement::Expression(_, expr) = &expressions.elements[0] {
assert_eq!(expr.expr, Expr::String(expected.to_string()))
} else {
panic!("Not an expression")

15
crates/nu-protocol/src/ast/block.rs
View File

@ -1,7 +1,6 @@
use super::Pipeline;
use crate::{ast::PipelineElement, Signature, Span, Type, VarId};
use serde::{Deserialize, Serialize};
use std::ops::{Index, IndexMut};
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct Block {
@ -22,20 +21,6 @@ impl Block {
}
}
impl Index<usize> for Block {
type Output = Pipeline;
fn index(&self, index: usize) -> &Self::Output {
&self.pipelines[index]
}
}
impl IndexMut<usize> for Block {
fn index_mut(&mut self, index: usize) -> &mut Self::Output {
&mut self.pipelines[index]
}
}
impl Default for Block {
fn default() -> Self {
Self::new()

15
crates/nu-protocol/src/ast/pipeline.rs
View File

@ -1,6 +1,5 @@
use crate::{ast::Expression, engine::StateWorkingSet, Span};
use serde::{Deserialize, Serialize};
use std::ops::{Index, IndexMut};
#[derive(Debug, Clone, Serialize, Deserialize, Eq, PartialEq)]
pub enum Redirection {
@ -159,17 +158,3 @@ impl Pipeline {
self.elements.is_empty()
}
}
impl Index<usize> for Pipeline {
type Output = PipelineElement;
fn index(&self, index: usize) -> &Self::Output {
&self.elements[index]
}
}
impl IndexMut<usize> for Pipeline {
fn index_mut(&mut self, index: usize) -> &mut Self::Output {
&mut self.elements[index]
}
}