Skip to content

Commit

Permalink
Fix workspace lint definition (#2120)
Browse files Browse the repository at this point in the history
* Small reduction in compile time

* Try workspace lints

* Fix the definition of lints at the workspace level, then all the code changes to avoid slice indexing in all the code!
  • Loading branch information
andrewdavidmackenzie authored Jan 4, 2024
1 parent bd68fbc commit a48ff1d
Show file tree
Hide file tree
Showing 87 changed files with 361 additions and 298 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,6 @@ unsafe_code = "forbid"
missing_docs = "deny"

[workspace.lints.clippy]
unwrap_used = "forbid"
unwrap_used = "deny"
result_large_err = "allow"
indexing_slicing = "deny"
3 changes: 3 additions & 0 deletions flowc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ readme = "README.md"
edition.workspace = true
lints.workspace = true

[lints]
workspace = true

[badges]
maintenance = { status = "actively-developed" }

Expand Down
36 changes: 24 additions & 12 deletions flowc/src/lib/compiler/gatherer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,8 +342,10 @@ mod test {
tables.connections = vec![only_connection];
collapse_connections(&mut tables).expect("Could not collapse connections");
assert_eq!(tables.collapsed_connections.len(), 1);
assert_eq!(*tables.collapsed_connections[0].from_io().route(), Route::from("/function1/out"));
assert_eq!(*tables.collapsed_connections[0].to_io().route(), Route::from("/function1/in"));
assert_eq!(*tables.collapsed_connections.first().expect("Could not get connection").from_io().route(),
Route::from("/function1/out"));
assert_eq!(*tables.collapsed_connections.first().expect("Could not get connection").to_io().route(),
Route::from("/function1/in"));
}

#[test]
Expand All @@ -363,8 +365,10 @@ mod test {
tables.connections = vec![only_connection];
collapse_connections(&mut tables).expect("Could not collapse connections");
assert_eq!(tables.collapsed_connections.len(), 1);
assert_eq!(*tables.collapsed_connections[0].from_io().route(), Route::from("/function1/out"));
assert_eq!(*tables.collapsed_connections[0].to_io().route(), Route::from("/function2/in"));
assert_eq!(*tables.collapsed_connections.first().expect("Could not get connection").from_io().route(),
Route::from("/function1/out"));
assert_eq!(*tables.collapsed_connections.first().expect("Could not get connection").to_io().route(),
Route::from("/function2/in"));
}

#[test]
Expand Down Expand Up @@ -407,8 +411,10 @@ mod test {
tables.connections = vec![left_side, extra_one, right_side];
collapse_connections(&mut tables).expect("Could not collapse connections");
assert_eq!(tables.collapsed_connections.len(), 1);
assert_eq!(*tables.collapsed_connections[0].from_io().route(), Route::from("/function1"));
assert_eq!(*tables.collapsed_connections[0].to_io().route(), Route::from("/flow2/function3"));
assert_eq!(*tables.collapsed_connections.first().expect("Could not get connection").from_io().route(),
Route::from("/function1"));
assert_eq!(*tables.collapsed_connections.first().expect("Could not get connection").to_io().route(),
Route::from("/flow2/function3"));
}

/*
Expand Down Expand Up @@ -454,11 +460,15 @@ mod test {
collapse_connections(&mut tables).expect("Could not collapse connections");
assert_eq!(2, tables.collapsed_connections.len());

assert_eq!(*tables.collapsed_connections[0].from_io().route(), Route::from("/f1"));
assert_eq!(*tables.collapsed_connections[0].to_io().route(), Route::from("/f2/value1"));
assert_eq!(*tables.collapsed_connections.first().expect("Could not get connection").from_io().route(),
Route::from("/f1"));
assert_eq!(*tables.collapsed_connections.first().expect("Could not get connection").to_io().route(),
Route::from("/f2/value1"));

assert_eq!(*tables.collapsed_connections[1].from_io().route(), Route::from("/f1"));
assert_eq!(*tables.collapsed_connections[1].to_io().route(), Route::from("/f2/value2"));
assert_eq!(*tables.collapsed_connections.get(1).expect("Could not get connection").from_io().route(),
Route::from("/f1"));
assert_eq!(*tables.collapsed_connections.get(1).expect("Could not get connection").to_io().route(),
Route::from("/f2/value2"));
}

#[test]
Expand Down Expand Up @@ -502,8 +512,10 @@ mod test {
collapse_connections(&mut tables).expect("Could not collapse connections");
assert_eq!(1, tables.collapsed_connections.len());

assert_eq!(*tables.collapsed_connections[0].from_io().route(), Route::from("/function1/out"));
assert_eq!(*tables.collapsed_connections[0].to_io().route(), Route::from("/flow1/flow2/func/in"));
assert_eq!(*tables.collapsed_connections.first().expect("Could not get connection").from_io().route(),
Route::from("/function1/out"));
assert_eq!(*tables.collapsed_connections.first().expect("Could not get connection").to_io().route(),
Route::from("/flow1/flow2/func/in"));
}

#[test]
Expand Down
40 changes: 22 additions & 18 deletions flowc/src/lib/dumper/flow_to_dot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,15 +151,15 @@ fn write_flow_to_dot(
contents.push_str(&input_set_to_dot(flow.inputs(), flow.route()));

// Outputs
contents.push_str(&output_set_to_dot(flow.outputs(), flow.route(), false));
contents.push_str(&output_set_to_dot(flow.outputs(), flow.route(), false)?);

// Process References
contents.push_str(&process_references_to_dot(flow)?);

// Connections
contents.push_str("\n\t// Connections");
for connection in &flow.connections {
contents.push_str(&connection_to_dot(connection));
contents.push_str(&connection_to_dot(connection)?);
}

dot_file.write_all(contents.as_bytes())?;
Expand Down Expand Up @@ -195,7 +195,7 @@ fn input_set_to_dot(input_set: &IOSet, to: &Route) -> String {
/*
Add the outputs from a flow to add points to connect to
*/
fn output_set_to_dot(output_set: &IOSet, from: &Route, connect_subflow: bool) -> String {
fn output_set_to_dot(output_set: &IOSet, from: &Route, connect_subflow: bool) -> Result<String> {
let mut string = String::new();

string.push_str("\n\t// Outputs\n\t{ rank=sink\n");
Expand All @@ -208,7 +208,7 @@ fn output_set_to_dot(output_set: &IOSet, from: &Route, connect_subflow: bool) ->

if connect_subflow {
// and connect the output to the sub-flow
let output_port = output_name_to_port(output.name());
let output_port = output_name_to_port(output.name())?;
let _ = writeln!(string,
"\t\"{}\":{} -> \"{}\"[style=invis, headtooltip=\"{}\"];",
from,
Expand All @@ -221,7 +221,7 @@ fn output_set_to_dot(output_set: &IOSet, from: &Route, connect_subflow: bool) ->
}
string.push_str("\t}\n");

string
Ok(string)
}

fn process_references_to_dot(flow: &FlowDefinition) -> Result<String> {
Expand Down Expand Up @@ -291,12 +291,12 @@ fn subfunction_to_dot(function: &FunctionDefinition, parent: PathBuf) -> Result<
name);
}

dot_string.push_str(&input_initializers_to_dot(function, function.route().as_ref()));
dot_string.push_str(&input_initializers_to_dot(function, function.route().as_ref())?);

Ok(dot_string)
}

pub (crate) fn input_initializers_to_dot(function: &FunctionDefinition, function_identifier: &str) -> String {
pub (crate) fn input_initializers_to_dot(function: &FunctionDefinition, function_identifier: &str) -> Result<String> {
let mut initializers = "\n\t// Initializers\n".to_string();

// TODO add initializers for sub-flows also
Expand All @@ -320,18 +320,18 @@ pub (crate) fn input_initializers_to_dot(function: &FunctionDefinition, function
"\t\"initializer{function_identifier}_{input_number}\" [style=invis];"
);

let input_port = input_name_to_port(input.name());
let input_port = input_name_to_port(input.name())?;

// Add connection from hidden node to the input being initialized
let _ = writeln!(initializers,
"\t\"initializer{function_identifier}_{input_number}\" -> \"{function_identifier}\":{input_port} [style={line_style}] [taillabel=\"{value_string}\"] [len=0.1] [color=blue];");
}
}

initializers
Ok(initializers)
}

fn connection_to_dot(connection: &Connection) -> String {
fn connection_to_dot(connection: &Connection) -> Result<String> {
// ensure no array index included in the source - just get the input route
let (from_route, number, array_index) =
connection.from_io().route().without_trailing_array_index();
Expand All @@ -341,7 +341,7 @@ fn connection_to_dot(connection: &Connection) -> String {
"", // connect from the "tip" of the flow input pentagon, no need for name
from_route.to_string())
} else {
(output_name_to_port(connection.from_io().name()),
(output_name_to_port(connection.from_io().name())?,
connection.from_io().name().as_str(),
from_route.parent(connection.from_io().name()))
};
Expand All @@ -352,19 +352,21 @@ fn connection_to_dot(connection: &Connection) -> String {
connection.to_io().route().to_string()
)
} else {
(input_name_to_port(connection.to_io().name()),
(input_name_to_port(connection.to_io().name())?,
connection.to_io().name().as_str(),
connection.to_io().route().parent(connection.to_io().name())
)
};

if array_index {
let output = if array_index {
format!(
"\n\t\"{from_node}\":{from_port} -> \"{to_node}\":{to_port} [xlabel=\"{from_name}[{number}]\", headlabel=\"{to_name}\"];")
} else {
format!(
"\n\t\"{from_node}\":{from_port} -> \"{to_node}\":{to_port} [xlabel=\"{from_name}\", headlabel=\"{to_name}\"];")
}
};

Ok(output)
}

fn digraph_start(flow: &FlowDefinition) -> String {
Expand Down Expand Up @@ -395,12 +397,14 @@ fn index_from_name<T: Hash>(t: &T, length: usize) -> usize {
index as usize
}

fn input_name_to_port<T: Hash>(t: &T) -> &str {
INPUT_PORTS[index_from_name(t, INPUT_PORTS.len())]
fn input_name_to_port<T: Hash>(t: &T) -> Result<&str> {
let index = index_from_name(t, INPUT_PORTS.len());
Ok(INPUT_PORTS.get(index).ok_or("Could not get input index")?)
}

pub(crate) fn output_name_to_port<T: Hash>(t: &T) -> &str {
OUTPUT_PORTS[index_from_name(t, OUTPUT_PORTS.len())]
pub(crate) fn output_name_to_port<T: Hash>(t: &T) -> Result<&str> {
Ok(OUTPUT_PORTS.get(index_from_name(t, OUTPUT_PORTS.len()))
.ok_or("Could not get output port")?)
}

// figure out a relative path to get to target from source
Expand Down
21 changes: 12 additions & 9 deletions flowc/src/lib/dumper/functions_to_dot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ fn process_refs_to_dot(
output.push_str("}\n");
}
FunctionProcess(ref function) => {
output_compiled_function(function.route(), tables, &mut output);
output_compiled_function(function.route(), tables, &mut output)?;
}
}
}
Expand All @@ -124,7 +124,7 @@ fn process_refs_to_dot(
}

// Given a Function as used in the code generation - generate a "dot" format string to draw it
fn function_to_dot(function: &FunctionDefinition, functions: &[FunctionDefinition]) -> String {
fn function_to_dot(function: &FunctionDefinition, functions: &[FunctionDefinition]) -> Result<String> {
let mut function_string = String::new();

// modify path to point to the .html page that's built from .md to document the function
Expand All @@ -141,13 +141,14 @@ fn function_to_dot(function: &FunctionDefinition, functions: &[FunctionDefinitio
function.get_id()
));

function_string.push_str(&input_initializers_to_dot(function, &format!("r{}", function.get_id())));
function_string.push_str(&input_initializers_to_dot(function, &format!("r{}", function.get_id()))?);

// Add edges for each of the outputs of this function to other ones
for destination in function.get_output_connections() {
let input_port = INPUT_PORTS[destination.destination_io_number % INPUT_PORTS.len()];
let destination_function = &functions[destination.destination_id];
let source_port = output_name_to_port(&destination.source);
let input_port = INPUT_PORTS.get(destination.destination_io_number % INPUT_PORTS.len())
.ok_or("Could no tget Input Port")?;
let destination_function = functions.get(destination.destination_id).ok_or("Could not get function")?;
let source_port = output_name_to_port(&destination.source)?;
let destination_name = destination_function
.get_inputs()
.get(destination.destination_io_number)
Expand All @@ -165,17 +166,19 @@ fn function_to_dot(function: &FunctionDefinition, functions: &[FunctionDefinitio
));
}

function_string
Ok(function_string)
}

fn output_compiled_function(
route: &Route,
tables: &CompilerTables,
output: &mut String,
) {
) -> Result<()>{
for function in &tables.functions {
if function.route() == route {
output.push_str(&function_to_dot(function, &tables.functions));
output.push_str(&function_to_dot(function, &tables.functions)?);
}
}

Ok(())
}
2 changes: 1 addition & 1 deletion flowc/src/lib/generator/generate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ fn function_to_runtimefunction(

let mut runtime_inputs = vec![];
for input in function.get_inputs() {
runtime_inputs.push(Input::from(input));
runtime_inputs.push(Input::try_from(input)?);
}

Ok(RuntimeFunction::new(
Expand Down
3 changes: 3 additions & 0 deletions flowcore/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,6 @@ serde_yaml = { version = "~0.9" }
[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
curl = {version = "~0.4" }
simpath = { version = "~2.5", features = ["urls"] }

[lints]
workspace = true
19 changes: 10 additions & 9 deletions flowcore/src/model/datatype.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,26 +172,27 @@ impl DataType {
}

/// Return the `DataType` for a Json `Value`, including nested values in arrays or maps
pub fn value_type(value: &Value) -> DataType {
pub(crate) fn value_type(value: &Value) -> Result<DataType> {
match value {
Value::String(_) => STRING_TYPE.into(),
Value::Bool(_) => BOOLEAN_TYPE.into(),
Value::Number(_) => NUMBER_TYPE.into(),
Value::String(_) => Ok(STRING_TYPE.into()),
Value::Bool(_) => Ok(BOOLEAN_TYPE.into()),
Value::Number(_) => Ok(NUMBER_TYPE.into()),
Value::Array(array) => {
if array.is_empty() {
DataType::from(format!("{ARRAY_TYPE}/{GENERIC_TYPE}"))
Ok(DataType::from(format!("{ARRAY_TYPE}/{GENERIC_TYPE}")))
} else {
DataType::from(format!("{ARRAY_TYPE}/{}", Self::value_type(&array[0])))
Ok(DataType::from(format!("{ARRAY_TYPE}/{}",
Self::value_type(array.first().ok_or("Could not get input")?)?)))
}
},
Value::Object(map) => {
if let Some(map_entry) = map.values().next() {
DataType::from(format!("{OBJECT_TYPE}/{}", Self::value_type(map_entry)))
Ok(DataType::from(format!("{OBJECT_TYPE}/{}", Self::value_type(map_entry)?)))
} else {
OBJECT_TYPE.into()
Ok(OBJECT_TYPE.into())
}
}
Value::Null => NULL_TYPE.into(),
Value::Null => Ok(NULL_TYPE.into()),
}
}

Expand Down
Loading

0 comments on commit a48ff1d

Please sign in to comment.