From 00c66707156b41fadcae01865850fe24ee4a4d65 Mon Sep 17 00:00:00 2001 From: JohnnyT Date: Mon, 18 Aug 2025 09:01:48 -0600 Subject: [PATCH 1/8] Refactors Validator --- lib/sc/document/validator.ex | 386 ------------------ lib/sc/interpreter.ex | 2 +- lib/sc/validator.ex | 76 ++++ lib/sc/validator/initial_state_validator.ex | 184 +++++++++ lib/sc/validator/reachability_analyzer.ex | 84 ++++ lib/sc/validator/state_validator.ex | 55 +++ lib/sc/validator/transition_validator.ex | 27 ++ lib/sc/validator/utils.ex | 83 ++++ test/sc/initial_states_test.exs | 10 +- test/sc/parser/hierarchy_test.exs | 2 +- test/sc/parser/parallel_parsing_test.exs | 4 +- .../edge_cases_test.exs} | 4 +- .../final_state_test.exs} | 10 +- .../state_types_test.exs} | 4 +- test/sc/{document => }/validator_test.exs | 4 +- 15 files changed, 529 insertions(+), 406 deletions(-) delete mode 100644 lib/sc/document/validator.ex create mode 100644 lib/sc/validator.ex create mode 100644 lib/sc/validator/initial_state_validator.ex create mode 100644 lib/sc/validator/reachability_analyzer.ex create mode 100644 lib/sc/validator/state_validator.ex create mode 100644 lib/sc/validator/transition_validator.ex create mode 100644 lib/sc/validator/utils.ex rename test/sc/{document/validator_edge_cases_test.exs => validator/edge_cases_test.exs} (98%) rename test/sc/{document/validator_final_state_test.exs => validator/final_state_test.exs} (89%) rename test/sc/{document/validator_state_types_test.exs => validator/state_types_test.exs} (95%) rename test/sc/{document => }/validator_test.exs (98%) diff --git a/lib/sc/document/validator.ex b/lib/sc/document/validator.ex deleted file mode 100644 index 2c1ef00..0000000 --- a/lib/sc/document/validator.ex +++ /dev/null @@ -1,386 +0,0 @@ -defmodule SC.Document.Validator do - @moduledoc """ - Validates SCXML documents for structural correctness and semantic consistency. - - Catches issues like invalid initial state references, unreachable states, - malformed hierarchies, and other problems that could cause runtime errors. - """ - - defstruct errors: [], warnings: [] - - @type validation_result :: %__MODULE__{ - errors: [String.t()], - warnings: [String.t()] - } - - @type validation_result_with_document :: {validation_result(), SC.Document.t()} - - @doc """ - Validate an SCXML document and optimize it for runtime use. - - Returns {:ok, optimized_document, warnings} if document is valid. - Returns {:error, errors, warnings} if document has validation errors. - The optimized document includes performance optimizations like lookup maps. - """ - @spec validate(SC.Document.t()) :: - {:ok, SC.Document.t(), [String.t()]} | {:error, [String.t()], [String.t()]} - def validate(%SC.Document{} = document) do - {result, final_document} = - %__MODULE__{} - |> validate_initial_state(document) - |> validate_state_ids(document) - |> validate_transition_targets(document) - |> validate_reachability(document) - |> finalize(document) - - case result.errors do - [] -> {:ok, final_document, result.warnings} - errors -> {:error, errors, result.warnings} - end - end - - @doc """ - Finalize validation with whole-document validations and optimization. - - This callback is called after all individual validations have completed, - allowing for validations that require the entire document context. - If the document is valid, it will be optimized for runtime performance. - """ - @spec finalize(validation_result(), SC.Document.t()) :: validation_result_with_document() - def finalize(%__MODULE__{} = result, %SC.Document{} = document) do - validated_result = - result - |> validate_hierarchical_consistency(document) - |> validate_initial_state_hierarchy(document) - - final_document = - case validated_result.errors do - [] -> - # Only optimize valid documents (state types already determined at parse time) - SC.Document.build_lookup_maps(document) - - _errors -> - # Don't waste time optimizing invalid documents - document - end - - {validated_result, final_document} - end - - # Validate that the document's initial state exists - defp validate_initial_state(%__MODULE__{} = result, %SC.Document{initial: nil}) do - # No initial state specified - this is valid, first state becomes initial - result - end - - defp validate_initial_state(%__MODULE__{} = result, %SC.Document{initial: initial} = document) do - if state_exists?(initial, document) do - result - else - add_error(result, "Initial state '#{initial}' does not exist") - end - end - - # Validate that all state IDs are unique and non-empty - defp validate_state_ids(%__MODULE__{} = result, %SC.Document{} = document) do - all_states = collect_all_states(document) - - result - |> validate_unique_ids(all_states) - |> validate_non_empty_ids(all_states) - end - - defp validate_unique_ids(%__MODULE__{} = result, states) do - ids = Enum.map(states, & &1.id) - duplicates = ids -- Enum.uniq(ids) - - case Enum.uniq(duplicates) do - [] -> - result - - dups -> - Enum.reduce(dups, result, fn dup, acc -> - add_error(acc, "Duplicate state ID '#{dup}'") - end) - end - end - - defp validate_non_empty_ids(%__MODULE__{} = result, states) do - empty_ids = Enum.filter(states, &(is_nil(&1.id) or &1.id == "")) - - Enum.reduce(empty_ids, result, fn _state, acc -> - add_error(acc, "State found with empty or nil ID") - end) - end - - # Validate that all transition targets exist - defp validate_transition_targets(%__MODULE__{} = result, %SC.Document{} = document) do - all_states = collect_all_states(document) - all_transitions = collect_all_transitions(all_states) - - Enum.reduce(all_transitions, result, fn transition, acc -> - if transition.target && !state_exists?(transition.target, document) do - add_error(acc, "Transition target '#{transition.target}' does not exist") - else - acc - end - end) - end - - # Validate that all states except the initial state are reachable - defp validate_reachability(%__MODULE__{} = result, %SC.Document{} = document) do - all_states = collect_all_states(document) - initial_state = get_initial_state(document) - - case initial_state do - nil when all_states != [] -> - # No initial state and we have states - first state becomes initial - reachable = find_reachable_states(hd(all_states).id, document, MapSet.new()) - validate_all_reachable(result, all_states, reachable) - - nil -> - # Empty document is valid - result - - initial -> - reachable = find_reachable_states(initial, document, MapSet.new()) - validate_all_reachable(result, all_states, reachable) - end - end - - defp validate_all_reachable(%__MODULE__{} = result, all_states, reachable) do - unreachable = - Enum.filter(all_states, fn state -> - !MapSet.member?(reachable, state.id) - end) - - Enum.reduce(unreachable, result, fn state, acc -> - add_warning(acc, "State '#{state.id}' is unreachable from initial state") - end) - end - - # Helper functions - - defp add_error(%__MODULE__{} = result, error) do - %{result | errors: [error | result.errors]} - end - - defp add_warning(%__MODULE__{} = result, warning) do - %{result | warnings: [warning | result.warnings]} - end - - defp state_exists?(state_id, %SC.Document{} = document) do - collect_all_states(document) - |> Enum.any?(&(&1.id == state_id)) - end - - defp collect_all_states(%SC.Document{states: states}) do - collect_states_recursive(states) - end - - defp collect_states_recursive(states) do - Enum.flat_map(states, fn state -> - [state | collect_states_recursive(state.states)] - end) - end - - defp collect_all_transitions(states) do - Enum.flat_map(states, fn state -> - state.transitions - end) - end - - defp get_initial_state(%SC.Document{initial: initial}) when is_binary(initial), do: initial - defp get_initial_state(%SC.Document{states: [first_state | _rest]}), do: first_state.id - defp get_initial_state(%SC.Document{states: []}), do: nil - - defp find_reachable_states(state_id, document, visited) do - if MapSet.member?(visited, state_id) do - visited - else - visited = MapSet.put(visited, state_id) - process_state_reachability(state_id, document, visited) - end - end - - defp process_state_reachability(state_id, document, visited) do - case find_state_by_id_linear(state_id, document) do - nil -> visited - state -> process_state_children_and_transitions(state, document, visited) - end - end - - defp process_state_children_and_transitions(state, document, visited) do - # Also mark child states as reachable if this is a compound state - child_visited = mark_child_states_reachable(state.states, document, visited) - - # Then process transitions - state.transitions - |> Enum.filter(& &1.target) - |> Enum.reduce(child_visited, fn transition, acc -> - find_reachable_states(transition.target, document, acc) - end) - end - - defp mark_child_states_reachable(child_states, document, visited) do - Enum.reduce(child_states, visited, fn child_state, acc -> - find_reachable_states(child_state.id, document, acc) - end) - end - - # Linear search for states during validation (before lookup maps are built) - defp find_state_by_id_linear(state_id, %SC.Document{} = document) do - collect_all_states(document) - |> Enum.find(&(&1.id == state_id)) - end - - # Finalize validation functions - - # Validate that hierarchical state references are consistent - defp validate_hierarchical_consistency(%__MODULE__{} = result, %SC.Document{} = document) do - all_states = collect_all_states(document) - - Enum.reduce(all_states, result, fn state, acc -> - validate_compound_state_initial(acc, state, document) - end) - end - - # Validate that compound states with initial attributes reference valid child states - defp validate_compound_state_initial( - %__MODULE__{} = result, - %SC.State{initial: nil} = state, - _document - ) do - # No initial attribute - check for initial element validation - validate_initial_element(result, state) - end - - defp validate_compound_state_initial( - %__MODULE__{} = result, - %SC.State{initial: initial_id} = state, - _document - ) do - result - # First check if state has both initial attribute and initial element (invalid) - |> validate_no_conflicting_initial_specs(state) - # Then validate the initial attribute reference - |> validate_initial_attribute_reference(state, initial_id) - end - - # Validate that a state doesn't have both initial attribute and initial element - defp validate_no_conflicting_initial_specs( - %__MODULE__{} = result, - %SC.State{initial: initial_attr} = state - ) do - has_initial_element = Enum.any?(state.states, &(&1.type == :initial)) - - if initial_attr && has_initial_element do - add_error( - result, - "State '#{state.id}' cannot have both initial attribute and initial element - use one or the other" - ) - else - result - end - end - - # Validate the initial attribute reference - defp validate_initial_attribute_reference( - %__MODULE__{} = result, - %SC.State{} = state, - initial_id - ) do - # Check if the initial state is a direct child of this compound state - if Enum.any?(state.states, &(&1.id == initial_id)) do - result - else - add_error( - result, - "State '#{state.id}' specifies initial='#{initial_id}' but '#{initial_id}' is not a direct child" - ) - end - end - - # Validate initial element constraints - defp validate_initial_element(%__MODULE__{} = result, %SC.State{} = state) do - case Enum.filter(state.states, &(&1.type == :initial)) do - [] -> - # No initial element - that's fine - result - - [initial_element] -> - validate_single_initial_element(result, state, initial_element) - - multiple_initial_elements -> - add_error( - result, - "State '#{state.id}' cannot have multiple initial elements - found #{length(multiple_initial_elements)}" - ) - end - end - - # Validate a single initial element - defp validate_single_initial_element( - %__MODULE__{} = result, - %SC.State{} = parent_state, - %SC.State{type: :initial} = initial_element - ) do - case initial_element.transitions do - [] -> - add_error( - result, - "Initial element in state '#{parent_state.id}' must contain exactly one transition" - ) - - [transition] -> - validate_initial_transition(result, parent_state, transition) - - multiple_transitions -> - add_error( - result, - "Initial element in state '#{parent_state.id}' must contain exactly one transition - found #{length(multiple_transitions)}" - ) - end - end - - # Validate the transition within an initial element - defp validate_initial_transition( - %__MODULE__{} = result, - %SC.State{} = parent_state, - %SC.Transition{} = transition - ) do - cond do - is_nil(transition.target) -> - add_error( - result, - "Initial element transition in state '#{parent_state.id}' must have a target" - ) - - not Enum.any?(parent_state.states, &(&1.id == transition.target && &1.type != :initial)) -> - add_error( - result, - "Initial element transition in state '#{parent_state.id}' targets '#{transition.target}' which is not a valid direct child" - ) - - true -> - result - end - end - - # Validate that if document has initial state, it must be a top-level state (not nested) - defp validate_initial_state_hierarchy(%__MODULE__{} = result, %SC.Document{initial: nil}) do - result - end - - defp validate_initial_state_hierarchy( - %__MODULE__{} = result, - %SC.Document{initial: initial_id} = document - ) do - # Check if initial state is a direct child of the document (top-level) - if Enum.any?(document.states, &(&1.id == initial_id)) do - result - else - add_warning(result, "Document initial state '#{initial_id}' is not a top-level state") - end - end -end diff --git a/lib/sc/interpreter.ex b/lib/sc/interpreter.ex index 79f7282..0a9533d 100644 --- a/lib/sc/interpreter.ex +++ b/lib/sc/interpreter.ex @@ -15,7 +15,7 @@ defmodule SC.Interpreter do """ @spec initialize(Document.t()) :: {:ok, StateChart.t()} | {:error, [String.t()], [String.t()]} def initialize(%Document{} = document) do - case Document.Validator.validate(document) do + case SC.Validator.validate(document) do {:ok, optimized_document, warnings} -> state_chart = StateChart.new(optimized_document, get_initial_configuration(optimized_document)) diff --git a/lib/sc/validator.ex b/lib/sc/validator.ex new file mode 100644 index 0000000..901a846 --- /dev/null +++ b/lib/sc/validator.ex @@ -0,0 +1,76 @@ +defmodule SC.Validator do + @moduledoc """ + Validates SCXML documents for structural correctness and semantic consistency. + + Catches issues like invalid initial state references, unreachable states, + malformed hierarchies, and other problems that could cause runtime errors. + """ + + alias SC.Validator.{ + InitialStateValidator, + ReachabilityAnalyzer, + StateValidator, + TransitionValidator + } + + defstruct errors: [], warnings: [] + + @type validation_result :: %__MODULE__{ + errors: [String.t()], + warnings: [String.t()] + } + + @type validation_result_with_document :: {validation_result(), SC.Document.t()} + + @doc """ + Validate an SCXML document and optimize it for runtime use. + + Returns {:ok, optimized_document, warnings} if document is valid. + Returns {:error, errors, warnings} if document has validation errors. + The optimized document includes performance optimizations like lookup maps. + """ + @spec validate(SC.Document.t()) :: + {:ok, SC.Document.t(), [String.t()]} | {:error, [String.t()], [String.t()]} + def validate(%SC.Document{} = document) do + {result, final_document} = + %__MODULE__{} + |> InitialStateValidator.validate_initial_state(document) + |> StateValidator.validate_state_ids(document) + |> TransitionValidator.validate_transition_targets(document) + |> ReachabilityAnalyzer.validate_reachability(document) + |> finalize(document) + + case result.errors do + [] -> {:ok, final_document, result.warnings} + errors -> {:error, errors, result.warnings} + end + end + + @doc """ + Finalize validation with whole-document validations and optimization. + + This callback is called after all individual validations have completed, + allowing for validations that require the entire document context. + If the document is valid, it will be optimized for runtime performance. + """ + @spec finalize(validation_result(), SC.Document.t()) :: validation_result_with_document() + def finalize(%__MODULE__{} = result, %SC.Document{} = document) do + validated_result = + result + |> InitialStateValidator.validate_hierarchical_consistency(document) + |> InitialStateValidator.validate_initial_state_hierarchy(document) + + final_document = + case validated_result.errors do + [] -> + # Only optimize valid documents (state types already determined at parse time) + SC.Document.build_lookup_maps(document) + + _errors -> + # Don't waste time optimizing invalid documents + document + end + + {validated_result, final_document} + end +end diff --git a/lib/sc/validator/initial_state_validator.ex b/lib/sc/validator/initial_state_validator.ex new file mode 100644 index 0000000..39797d7 --- /dev/null +++ b/lib/sc/validator/initial_state_validator.ex @@ -0,0 +1,184 @@ +defmodule SC.Validator.InitialStateValidator do + @moduledoc """ + Validates initial state constraints in SCXML documents. + + Handles document-level initial states, compound state initial attributes, + initial elements, and hierarchical consistency validation. + """ + + alias SC.Validator.Utils + + @doc """ + Validate that the document's initial state exists. + """ + @spec validate_initial_state(SC.Validator.validation_result(), SC.Document.t()) :: + SC.Validator.validation_result() + def validate_initial_state(%SC.Validator{} = result, %SC.Document{initial: nil}) do + # No initial state specified - this is valid, first state becomes initial + result + end + + def validate_initial_state(%SC.Validator{} = result, %SC.Document{initial: initial} = document) do + if Utils.state_exists?(initial, document) do + result + else + Utils.add_error(result, "Initial state '#{initial}' does not exist") + end + end + + @doc """ + Validate that hierarchical state references are consistent. + """ + @spec validate_hierarchical_consistency(SC.Validator.validation_result(), SC.Document.t()) :: + SC.Validator.validation_result() + def validate_hierarchical_consistency(%SC.Validator{} = result, %SC.Document{} = document) do + all_states = Utils.collect_all_states(document) + + Enum.reduce(all_states, result, fn state, acc -> + validate_compound_state_initial(acc, state, document) + end) + end + + @doc """ + Validate that if document has initial state, it must be a top-level state (not nested). + """ + @spec validate_initial_state_hierarchy(SC.Validator.validation_result(), SC.Document.t()) :: + SC.Validator.validation_result() + def validate_initial_state_hierarchy(%SC.Validator{} = result, %SC.Document{initial: nil}) do + result + end + + def validate_initial_state_hierarchy( + %SC.Validator{} = result, + %SC.Document{initial: initial_id} = document + ) do + # Check if initial state is a direct child of the document (top-level) + if Enum.any?(document.states, &(&1.id == initial_id)) do + result + else + Utils.add_warning(result, "Document initial state '#{initial_id}' is not a top-level state") + end + end + + # Validate that compound states with initial attributes reference valid child states + defp validate_compound_state_initial( + %SC.Validator{} = result, + %SC.State{initial: nil} = state, + _document + ) do + # No initial attribute - check for initial element validation + validate_initial_element(result, state) + end + + defp validate_compound_state_initial( + %SC.Validator{} = result, + %SC.State{initial: initial_id} = state, + _document + ) do + result + # First check if state has both initial attribute and initial element (invalid) + |> validate_no_conflicting_initial_specs(state) + # Then validate the initial attribute reference + |> validate_initial_attribute_reference(state, initial_id) + end + + # Validate that a state doesn't have both initial attribute and initial element + defp validate_no_conflicting_initial_specs( + %SC.Validator{} = result, + %SC.State{initial: initial_attr} = state + ) do + has_initial_element = Enum.any?(state.states, &(&1.type == :initial)) + + if initial_attr && has_initial_element do + Utils.add_error( + result, + "State '#{state.id}' cannot have both initial attribute and initial element - use one or the other" + ) + else + result + end + end + + # Validate the initial attribute reference + defp validate_initial_attribute_reference( + %SC.Validator{} = result, + %SC.State{} = state, + initial_id + ) do + # Check if the initial state is a direct child of this compound state + if Enum.any?(state.states, &(&1.id == initial_id)) do + result + else + Utils.add_error( + result, + "State '#{state.id}' specifies initial='#{initial_id}' but '#{initial_id}' is not a direct child" + ) + end + end + + # Validate initial element constraints + defp validate_initial_element(%SC.Validator{} = result, %SC.State{} = state) do + case Enum.filter(state.states, &(&1.type == :initial)) do + [] -> + # No initial element - that's fine + result + + [initial_element] -> + validate_single_initial_element(result, state, initial_element) + + multiple_initial_elements -> + Utils.add_error( + result, + "State '#{state.id}' cannot have multiple initial elements - found #{length(multiple_initial_elements)}" + ) + end + end + + # Validate a single initial element + defp validate_single_initial_element( + %SC.Validator{} = result, + %SC.State{} = parent_state, + %SC.State{type: :initial} = initial_element + ) do + case initial_element.transitions do + [] -> + Utils.add_error( + result, + "Initial element in state '#{parent_state.id}' must contain exactly one transition" + ) + + [transition] -> + validate_initial_transition(result, parent_state, transition) + + multiple_transitions -> + Utils.add_error( + result, + "Initial element in state '#{parent_state.id}' must contain exactly one transition - found #{length(multiple_transitions)}" + ) + end + end + + # Validate the transition within an initial element + defp validate_initial_transition( + %SC.Validator{} = result, + %SC.State{} = parent_state, + %SC.Transition{} = transition + ) do + cond do + is_nil(transition.target) -> + Utils.add_error( + result, + "Initial element transition in state '#{parent_state.id}' must have a target" + ) + + not Enum.any?(parent_state.states, &(&1.id == transition.target && &1.type != :initial)) -> + Utils.add_error( + result, + "Initial element transition in state '#{parent_state.id}' targets '#{transition.target}' which is not a valid direct child" + ) + + true -> + result + end + end +end diff --git a/lib/sc/validator/reachability_analyzer.ex b/lib/sc/validator/reachability_analyzer.ex new file mode 100644 index 0000000..989f35e --- /dev/null +++ b/lib/sc/validator/reachability_analyzer.ex @@ -0,0 +1,84 @@ +defmodule SC.Validator.ReachabilityAnalyzer do + @moduledoc """ + Analyzes state reachability in SCXML documents. + + Performs graph traversal to identify unreachable states and warn about them. + """ + + alias SC.Validator.Utils + + @doc """ + Validate that all states except the initial state are reachable. + """ + @spec validate_reachability(SC.Validator.validation_result(), SC.Document.t()) :: + SC.Validator.validation_result() + def validate_reachability(%SC.Validator{} = result, %SC.Document{} = document) do + all_states = Utils.collect_all_states(document) + initial_state = Utils.get_initial_state(document) + + case initial_state do + nil when all_states != [] -> + # No initial state and we have states - first state becomes initial + reachable = find_reachable_states(hd(all_states).id, document, MapSet.new()) + validate_all_reachable(result, all_states, reachable) + + nil -> + # Empty document is valid + result + + initial -> + reachable = find_reachable_states(initial, document, MapSet.new()) + validate_all_reachable(result, all_states, reachable) + end + end + + @doc """ + Find all states reachable from a given starting state. + """ + @spec find_reachable_states(String.t(), SC.Document.t(), MapSet.t(String.t())) :: + MapSet.t(String.t()) + def find_reachable_states(state_id, document, visited) do + if MapSet.member?(visited, state_id) do + visited + else + visited = MapSet.put(visited, state_id) + process_state_reachability(state_id, document, visited) + end + end + + defp validate_all_reachable(%SC.Validator{} = result, all_states, reachable) do + unreachable = + Enum.filter(all_states, fn state -> + !MapSet.member?(reachable, state.id) + end) + + Enum.reduce(unreachable, result, fn state, acc -> + Utils.add_warning(acc, "State '#{state.id}' is unreachable from initial state") + end) + end + + defp process_state_reachability(state_id, document, visited) do + case Utils.find_state_by_id_linear(state_id, document) do + nil -> visited + state -> process_state_children_and_transitions(state, document, visited) + end + end + + defp process_state_children_and_transitions(state, document, visited) do + # Also mark child states as reachable if this is a compound state + child_visited = mark_child_states_reachable(state.states, document, visited) + + # Then process transitions + state.transitions + |> Enum.filter(& &1.target) + |> Enum.reduce(child_visited, fn transition, acc -> + find_reachable_states(transition.target, document, acc) + end) + end + + defp mark_child_states_reachable(child_states, document, visited) do + Enum.reduce(child_states, visited, fn child_state, acc -> + find_reachable_states(child_state.id, document, acc) + end) + end +end diff --git a/lib/sc/validator/state_validator.ex b/lib/sc/validator/state_validator.ex new file mode 100644 index 0000000..614b0ce --- /dev/null +++ b/lib/sc/validator/state_validator.ex @@ -0,0 +1,55 @@ +defmodule SC.Validator.StateValidator do + @moduledoc """ + Validates state-related constraints in SCXML documents. + + Handles state ID uniqueness, non-empty IDs, and basic state structure validation. + """ + + alias SC.Validator.Utils + + @doc """ + Validate that all state IDs are unique and non-empty. + """ + @spec validate_state_ids(SC.Validator.validation_result(), SC.Document.t()) :: + SC.Validator.validation_result() + def validate_state_ids(%SC.Validator{} = result, %SC.Document{} = document) do + all_states = Utils.collect_all_states(document) + + result + |> validate_unique_ids(all_states) + |> validate_non_empty_ids(all_states) + end + + @doc """ + Validate that all state IDs are unique within the document. + """ + @spec validate_unique_ids(SC.Validator.validation_result(), [SC.State.t()]) :: + SC.Validator.validation_result() + def validate_unique_ids(%SC.Validator{} = result, states) do + ids = Enum.map(states, & &1.id) + duplicates = ids -- Enum.uniq(ids) + + case Enum.uniq(duplicates) do + [] -> + result + + dups -> + Enum.reduce(dups, result, fn dup, acc -> + Utils.add_error(acc, "Duplicate state ID '#{dup}'") + end) + end + end + + @doc """ + Validate that no states have empty or nil IDs. + """ + @spec validate_non_empty_ids(SC.Validator.validation_result(), [SC.State.t()]) :: + SC.Validator.validation_result() + def validate_non_empty_ids(%SC.Validator{} = result, states) do + empty_ids = Enum.filter(states, &(is_nil(&1.id) or &1.id == "")) + + Enum.reduce(empty_ids, result, fn _state, acc -> + Utils.add_error(acc, "State found with empty or nil ID") + end) + end +end diff --git a/lib/sc/validator/transition_validator.ex b/lib/sc/validator/transition_validator.ex new file mode 100644 index 0000000..c45ff7a --- /dev/null +++ b/lib/sc/validator/transition_validator.ex @@ -0,0 +1,27 @@ +defmodule SC.Validator.TransitionValidator do + @moduledoc """ + Validates transition-related constraints in SCXML documents. + + Handles transition target validation and transition structure validation. + """ + + alias SC.Validator.Utils + + @doc """ + Validate that all transition targets exist in the document. + """ + @spec validate_transition_targets(SC.Validator.validation_result(), SC.Document.t()) :: + SC.Validator.validation_result() + def validate_transition_targets(%SC.Validator{} = result, %SC.Document{} = document) do + all_states = Utils.collect_all_states(document) + all_transitions = Utils.collect_all_transitions(all_states) + + Enum.reduce(all_transitions, result, fn transition, acc -> + if transition.target && !Utils.state_exists?(transition.target, document) do + Utils.add_error(acc, "Transition target '#{transition.target}' does not exist") + else + acc + end + end) + end +end diff --git a/lib/sc/validator/utils.ex b/lib/sc/validator/utils.ex new file mode 100644 index 0000000..54c3b5d --- /dev/null +++ b/lib/sc/validator/utils.ex @@ -0,0 +1,83 @@ +defmodule SC.Validator.Utils do + @moduledoc """ + Utility functions shared across validator modules. + + Provides common operations like result manipulation, state collection, + and document traversal helpers. + """ + + @doc """ + Add an error to the validation result. + """ + @spec add_error(SC.Validator.validation_result(), String.t()) :: + SC.Validator.validation_result() + def add_error(%SC.Validator{} = result, error) do + %{result | errors: [error | result.errors]} + end + + @doc """ + Add a warning to the validation result. + """ + @spec add_warning(SC.Validator.validation_result(), String.t()) :: + SC.Validator.validation_result() + def add_warning(%SC.Validator{} = result, warning) do + %{result | warnings: [warning | result.warnings]} + end + + @doc """ + Check if a state with the given ID exists in the document. + """ + @spec state_exists?(String.t(), SC.Document.t()) :: boolean() + def state_exists?(state_id, %SC.Document{} = document) do + collect_all_states(document) + |> Enum.any?(&(&1.id == state_id)) + end + + @doc """ + Collect all states from a document, including nested states. + """ + @spec collect_all_states(SC.Document.t()) :: [SC.State.t()] + def collect_all_states(%SC.Document{states: states}) do + collect_states_recursive(states) + end + + @doc """ + Recursively collect states from a state list. + """ + @spec collect_states_recursive([SC.State.t()]) :: [SC.State.t()] + def collect_states_recursive(states) do + Enum.flat_map(states, fn state -> + [state | collect_states_recursive(state.states)] + end) + end + + @doc """ + Collect all transitions from a list of states. + """ + @spec collect_all_transitions([SC.State.t()]) :: [SC.Transition.t()] + def collect_all_transitions(states) do + Enum.flat_map(states, fn state -> + state.transitions + end) + end + + @doc """ + Get the initial state ID from a document. + + Returns the explicit initial state if set, otherwise the first state's ID, + or nil if the document has no states. + """ + @spec get_initial_state(SC.Document.t()) :: String.t() | nil + def get_initial_state(%SC.Document{initial: initial}) when is_binary(initial), do: initial + def get_initial_state(%SC.Document{states: [first_state | _rest]}), do: first_state.id + def get_initial_state(%SC.Document{states: []}), do: nil + + @doc """ + Linear search for a state by ID during validation (before lookup maps are built). + """ + @spec find_state_by_id_linear(String.t(), SC.Document.t()) :: SC.State.t() | nil + def find_state_by_id_linear(state_id, %SC.Document{} = document) do + collect_all_states(document) + |> Enum.find(&(&1.id == state_id)) + end +end diff --git a/test/sc/initial_states_test.exs b/test/sc/initial_states_test.exs index 5bf0c7d..38dbe70 100644 --- a/test/sc/initial_states_test.exs +++ b/test/sc/initial_states_test.exs @@ -120,7 +120,7 @@ defmodule SC.InitialStatesTest do """ {:ok, document} = Parser.SCXML.parse(xml) - {:ok, _document, warnings} = Document.Validator.validate(document) + {:ok, _document, warnings} = SC.Validator.validate(document) assert Enum.empty?(warnings) end @@ -138,7 +138,7 @@ defmodule SC.InitialStatesTest do """ {:ok, document} = Parser.SCXML.parse(xml) - {:error, errors, _warnings} = Document.Validator.validate(document) + {:error, errors, _warnings} = SC.Validator.validate(document) assert length(errors) == 1 assert hd(errors) =~ "cannot have both initial attribute and initial element" @@ -158,7 +158,7 @@ defmodule SC.InitialStatesTest do } document = %Document{states: [compound_state]} - {:error, errors, _warnings} = Document.Validator.validate(document) + {:error, errors, _warnings} = SC.Validator.validate(document) assert length(errors) >= 1 assert Enum.any?(errors, &String.contains?(&1, "must contain exactly one transition")) @@ -180,7 +180,7 @@ defmodule SC.InitialStatesTest do } document = %Document{states: [compound_state]} - {:error, errors, _warnings} = Document.Validator.validate(document) + {:error, errors, _warnings} = SC.Validator.validate(document) assert length(errors) >= 1 assert Enum.any?(errors, &String.contains?(&1, "not a valid direct child")) @@ -198,7 +198,7 @@ defmodule SC.InitialStatesTest do } document = %Document{states: [compound_state]} - {:error, errors, _warnings} = Document.Validator.validate(document) + {:error, errors, _warnings} = SC.Validator.validate(document) assert length(errors) >= 1 assert Enum.any?(errors, &String.contains?(&1, "cannot have multiple initial elements")) diff --git a/test/sc/parser/hierarchy_test.exs b/test/sc/parser/hierarchy_test.exs index 7c7e321..550868b 100644 --- a/test/sc/parser/hierarchy_test.exs +++ b/test/sc/parser/hierarchy_test.exs @@ -57,7 +57,7 @@ defmodule SC.Parser.HierarchyTest do """ {:ok, raw_document} = SCXML.parse(xml) - {:ok, document, _warnings} = Document.Validator.validate(raw_document) + {:ok, document, _warnings} = SC.Validator.validate(raw_document) # Create a configuration with the deepest state active config = SC.Configuration.new(["grandchild"]) diff --git a/test/sc/parser/parallel_parsing_test.exs b/test/sc/parser/parallel_parsing_test.exs index 92291d4..6caf135 100644 --- a/test/sc/parser/parallel_parsing_test.exs +++ b/test/sc/parser/parallel_parsing_test.exs @@ -43,7 +43,7 @@ defmodule SC.Parser.ParallelParsingTest do """ {:ok, document} = SCXML.parse(xml) - {:ok, validated_document, _warnings} = Document.Validator.validate(document) + {:ok, validated_document, _warnings} = SC.Validator.validate(document) [parallel_state] = validated_document.states assert parallel_state.type == :parallel @@ -69,7 +69,7 @@ defmodule SC.Parser.ParallelParsingTest do """ {:ok, document} = SCXML.parse(xml) - {:ok, validated_document, _warnings} = Document.Validator.validate(document) + {:ok, validated_document, _warnings} = SC.Validator.validate(document) [parallel_state] = validated_document.states assert parallel_state.type == :parallel diff --git a/test/sc/document/validator_edge_cases_test.exs b/test/sc/validator/edge_cases_test.exs similarity index 98% rename from test/sc/document/validator_edge_cases_test.exs rename to test/sc/validator/edge_cases_test.exs index fa3ca3e..350a13b 100644 --- a/test/sc/document/validator_edge_cases_test.exs +++ b/test/sc/validator/edge_cases_test.exs @@ -1,7 +1,7 @@ -defmodule SC.Document.ValidatorEdgeCasesTest do +defmodule SC.Validator.EdgeCasesTest do use ExUnit.Case - alias SC.Document.Validator + alias SC.Validator alias SC.{Document, State, Transition} describe "error handling edge cases" do diff --git a/test/sc/document/validator_final_state_test.exs b/test/sc/validator/final_state_test.exs similarity index 89% rename from test/sc/document/validator_final_state_test.exs rename to test/sc/validator/final_state_test.exs index 00ea389..e3e8c28 100644 --- a/test/sc/document/validator_final_state_test.exs +++ b/test/sc/validator/final_state_test.exs @@ -1,4 +1,4 @@ -defmodule SC.Document.ValidatorFinalStateTest do +defmodule SC.Validator.FinalStateTest do use ExUnit.Case alias SC.{Document, Parser.SCXML} @@ -15,7 +15,7 @@ defmodule SC.Document.ValidatorFinalStateTest do """ {:ok, document} = SCXML.parse(xml) - {:ok, validated_document, _warnings} = Document.Validator.validate(document) + {:ok, validated_document, _warnings} = SC.Validator.validate(document) # Find the final state in the validated document final_state = Enum.find(validated_document.states, &(&1.id == "final_state")) @@ -39,7 +39,7 @@ defmodule SC.Document.ValidatorFinalStateTest do """ {:ok, document} = SCXML.parse(xml) - {:ok, validated_document, _warnings} = Document.Validator.validate(document) + {:ok, validated_document, _warnings} = SC.Validator.validate(document) # Find the compound state compound_state = Enum.find(validated_document.states, &(&1.id == "compound")) @@ -64,7 +64,7 @@ defmodule SC.Document.ValidatorFinalStateTest do """ {:ok, document} = SCXML.parse(xml) - {:ok, validated_document, _warnings} = Document.Validator.validate(document) + {:ok, validated_document, _warnings} = SC.Validator.validate(document) # Find the parallel state parallel_state = Enum.find(validated_document.states, &(&1.id == "parallel_state")) @@ -91,7 +91,7 @@ defmodule SC.Document.ValidatorFinalStateTest do """ {:ok, document} = SCXML.parse(xml) - {:ok, validated_document, _warnings} = Document.Validator.validate(document) + {:ok, validated_document, _warnings} = SC.Validator.validate(document) # Check both final states are preserved final1 = Enum.find(validated_document.states, &(&1.id == "final1")) diff --git a/test/sc/document/validator_state_types_test.exs b/test/sc/validator/state_types_test.exs similarity index 95% rename from test/sc/document/validator_state_types_test.exs rename to test/sc/validator/state_types_test.exs index f89a83b..e209bdc 100644 --- a/test/sc/document/validator_state_types_test.exs +++ b/test/sc/validator/state_types_test.exs @@ -1,4 +1,4 @@ -defmodule SC.Document.ValidatorStateTypesTest do +defmodule SC.Validator.StateTypesTest do use ExUnit.Case, async: true alias SC.{Document, Parser.SCXML} @@ -48,7 +48,7 @@ defmodule SC.Document.ValidatorStateTypesTest do """ {:ok, document} = SCXML.parse(xml) - {:error, _errors, _warnings} = Document.Validator.validate(document) + {:error, _errors, _warnings} = SC.Validator.validate(document) # State types are determined at parse time regardless of validity [state] = document.states diff --git a/test/sc/document/validator_test.exs b/test/sc/validator_test.exs similarity index 98% rename from test/sc/document/validator_test.exs rename to test/sc/validator_test.exs index 5bd91a0..3b7bdeb 100644 --- a/test/sc/document/validator_test.exs +++ b/test/sc/validator_test.exs @@ -1,8 +1,8 @@ -defmodule SC.Document.ValidatorTest do +defmodule SC.ValidatorTest do use ExUnit.Case, async: true - alias SC.Document.Validator alias SC.Parser.SCXML + alias SC.Validator describe "validate/1" do test "validates simple valid document" do From d54bf4558a35c53afe3b363203db58bbf2a006df Mon Sep 17 00:00:00 2001 From: JohnnyT Date: Mon, 18 Aug 2025 09:12:31 -0600 Subject: [PATCH 2/8] Requires aliasing of modules --- .credo.exs | 2 +- lib/sc/configuration.ex | 6 ++- lib/sc/interpreter.ex | 4 +- lib/sc/state_chart.ex | 12 +++--- lib/sc/validator.ex | 4 +- test/sc/feature_detector_test.exs | 15 +++---- test/sc/initial_states_test.exs | 44 +++++++++++---------- test/sc/interpreter/compound_state_test.exs | 5 ++- test/sc/parser/hierarchy_test.exs | 9 +++-- test/sc/parser/parallel_parsing_test.exs | 7 ++-- test/sc/validator/final_state_test.exs | 11 +++--- test/sc/validator/state_types_test.exs | 5 ++- test/support/sc_case.ex | 3 +- 13 files changed, 71 insertions(+), 56 deletions(-) diff --git a/.credo.exs b/.credo.exs index 63133c0..e533ac4 100644 --- a/.credo.exs +++ b/.credo.exs @@ -84,7 +84,7 @@ # Priority values are: `low, normal, high, higher` # {Credo.Check.Design.AliasUsage, - [priority: :low, if_nested_deeper_than: 2, if_called_more_often_than: 0]}, + [priority: :low, if_nested_deeper_than: 1, if_called_more_often_than: 0]}, {Credo.Check.Design.TagFIXME, []}, # You can also customize the exit_status of each check. # If you don't want TODO comments to cause `mix credo` to fail, just diff --git a/lib/sc/configuration.ex b/lib/sc/configuration.ex index 105457c..86cd2ee 100644 --- a/lib/sc/configuration.ex +++ b/lib/sc/configuration.ex @@ -7,6 +7,8 @@ defmodule SC.Configuration do the full set of active states including ancestors. """ + alias SC.Document + defstruct active_states: MapSet.new() @type t :: %__MODULE__{ @@ -73,7 +75,7 @@ defmodule SC.Configuration do # Fast O(d) ancestor lookup using parent pointers and O(1) state lookup defp get_state_ancestors(state_id, document) do - case SC.Document.find_state(document, state_id) do + case Document.find_state(document, state_id) do nil -> [] state -> collect_ancestors(state, document, []) end @@ -83,7 +85,7 @@ defmodule SC.Configuration do defp collect_ancestors(%SC.State{parent: nil}, _document, ancestors), do: ancestors defp collect_ancestors(%SC.State{parent: parent_id}, document, ancestors) do - case SC.Document.find_state(document, parent_id) do + case Document.find_state(document, parent_id) do nil -> ancestors parent_state -> collect_ancestors(parent_state, document, [parent_id | ancestors]) end diff --git a/lib/sc/interpreter.ex b/lib/sc/interpreter.ex index 0a9533d..f421752 100644 --- a/lib/sc/interpreter.ex +++ b/lib/sc/interpreter.ex @@ -6,7 +6,7 @@ defmodule SC.Interpreter do Documents are automatically validated before interpretation. """ - alias SC.{Configuration, Document, Event, StateChart} + alias SC.{Configuration, Document, Event, StateChart, Validator} @doc """ Initialize a state chart from a parsed document. @@ -15,7 +15,7 @@ defmodule SC.Interpreter do """ @spec initialize(Document.t()) :: {:ok, StateChart.t()} | {:error, [String.t()], [String.t()]} def initialize(%Document{} = document) do - case SC.Validator.validate(document) do + case Validator.validate(document) do {:ok, optimized_document, warnings} -> state_chart = StateChart.new(optimized_document, get_initial_configuration(optimized_document)) diff --git a/lib/sc/state_chart.ex b/lib/sc/state_chart.ex index acc482b..d52a884 100644 --- a/lib/sc/state_chart.ex +++ b/lib/sc/state_chart.ex @@ -6,13 +6,15 @@ defmodule SC.StateChart do for internal and external events as specified by the SCXML specification. """ + alias SC.{Configuration, Document, Event} + defstruct [:document, :configuration, internal_queue: [], external_queue: []] @type t :: %__MODULE__{ - document: SC.Document.t(), - configuration: SC.Configuration.t(), - internal_queue: [SC.Event.t()], - external_queue: [SC.Event.t()] + document: Document.t(), + configuration: Configuration.t(), + internal_queue: [Event.t()], + external_queue: [Event.t()] } @doc """ @@ -90,6 +92,6 @@ defmodule SC.StateChart do """ @spec active_states(t()) :: MapSet.t(String.t()) def active_states(%__MODULE__{} = state_chart) do - SC.Configuration.active_ancestors(state_chart.configuration, state_chart.document) + Configuration.active_ancestors(state_chart.configuration, state_chart.document) end end diff --git a/lib/sc/validator.ex b/lib/sc/validator.ex index 901a846..b5710a4 100644 --- a/lib/sc/validator.ex +++ b/lib/sc/validator.ex @@ -6,6 +6,8 @@ defmodule SC.Validator do malformed hierarchies, and other problems that could cause runtime errors. """ + alias SC.Document + alias SC.Validator.{ InitialStateValidator, ReachabilityAnalyzer, @@ -64,7 +66,7 @@ defmodule SC.Validator do case validated_result.errors do [] -> # Only optimize valid documents (state types already determined at parse time) - SC.Document.build_lookup_maps(document) + Document.build_lookup_maps(document) _errors -> # Don't waste time optimizing invalid documents diff --git a/test/sc/feature_detector_test.exs b/test/sc/feature_detector_test.exs index 422f5e2..8ec37e4 100644 --- a/test/sc/feature_detector_test.exs +++ b/test/sc/feature_detector_test.exs @@ -2,6 +2,7 @@ defmodule SC.FeatureDetectorTest do use ExUnit.Case alias SC.{Document, FeatureDetector, Parser, State} + alias SC.Parser.SCXML describe "feature detection from XML" do test "detects basic states and transitions" do @@ -246,7 +247,7 @@ defmodule SC.FeatureDetectorTest do """ - {:ok, document} = Parser.SCXML.parse(xml) + {:ok, document} = SCXML.parse(xml) features = FeatureDetector.detect_features(document) assert MapSet.member?(features, :basic_states) @@ -264,7 +265,7 @@ defmodule SC.FeatureDetectorTest do """ - {:ok, document} = Parser.SCXML.parse(xml) + {:ok, document} = SCXML.parse(xml) features = FeatureDetector.detect_features(document) assert MapSet.member?(features, :basic_states) @@ -282,7 +283,7 @@ defmodule SC.FeatureDetectorTest do """ - {:ok, document} = Parser.SCXML.parse(xml) + {:ok, document} = SCXML.parse(xml) features = FeatureDetector.detect_features(document) assert MapSet.member?(features, :basic_states) @@ -314,7 +315,7 @@ defmodule SC.FeatureDetectorTest do """ - {:ok, document} = Parser.SCXML.parse(xml) + {:ok, document} = SCXML.parse(xml) features = FeatureDetector.detect_features(document) assert MapSet.member?(features, :basic_states) @@ -329,7 +330,7 @@ defmodule SC.FeatureDetectorTest do """ - {:ok, document} = Parser.SCXML.parse(xml) + {:ok, document} = SCXML.parse(xml) features = FeatureDetector.detect_features(document) assert MapSet.member?(features, :basic_states) @@ -347,7 +348,7 @@ defmodule SC.FeatureDetectorTest do """ - {:ok, document} = Parser.SCXML.parse(xml) + {:ok, document} = SCXML.parse(xml) features = FeatureDetector.detect_features(document) assert MapSet.member?(features, :basic_states) @@ -364,7 +365,7 @@ defmodule SC.FeatureDetectorTest do """ - {:ok, document} = Parser.SCXML.parse(xml) + {:ok, document} = SCXML.parse(xml) features = FeatureDetector.detect_features(document) assert MapSet.member?(features, :basic_states) diff --git a/test/sc/initial_states_test.exs b/test/sc/initial_states_test.exs index 38dbe70..c39eb3c 100644 --- a/test/sc/initial_states_test.exs +++ b/test/sc/initial_states_test.exs @@ -1,6 +1,8 @@ defmodule SC.InitialStatesTest do use ExUnit.Case - alias SC.{Document, Parser, State} + alias SC.{Document, Parser, State, Transition, Validator} + alias SC.{FeatureDetector, Interpreter} + alias SC.Parser.SCXML describe "initial element parsing" do test "parses initial element with transition" do @@ -16,7 +18,7 @@ defmodule SC.InitialStatesTest do """ - {:ok, document} = Parser.SCXML.parse(xml) + {:ok, document} = SCXML.parse(xml) # Check document structure and find the compound state assert length(document.states) > 0 @@ -46,7 +48,7 @@ defmodule SC.InitialStatesTest do """ - {:ok, document} = Parser.SCXML.parse(xml) + {:ok, document} = SCXML.parse(xml) # Check document structure and find the parallel state assert length(document.states) > 0 @@ -77,7 +79,7 @@ defmodule SC.InitialStatesTest do """ - features = SC.FeatureDetector.detect_features(xml) + features = FeatureDetector.detect_features(xml) assert MapSet.member?(features, :initial_elements) assert MapSet.member?(features, :basic_states) end @@ -94,14 +96,14 @@ defmodule SC.InitialStatesTest do """ - {:ok, document} = Parser.SCXML.parse(xml) - features = SC.FeatureDetector.detect_features(document) + {:ok, document} = SCXML.parse(xml) + features = FeatureDetector.detect_features(document) assert MapSet.member?(features, :initial_elements) end test "validates initial elements are supported" do features = MapSet.new([:basic_states, :initial_elements]) - assert {:ok, ^features} = SC.FeatureDetector.validate_features(features) + assert {:ok, ^features} = FeatureDetector.validate_features(features) end end @@ -119,8 +121,8 @@ defmodule SC.InitialStatesTest do """ - {:ok, document} = Parser.SCXML.parse(xml) - {:ok, _document, warnings} = SC.Validator.validate(document) + {:ok, document} = SCXML.parse(xml) + {:ok, _document, warnings} = Validator.validate(document) assert Enum.empty?(warnings) end @@ -137,8 +139,8 @@ defmodule SC.InitialStatesTest do """ - {:ok, document} = Parser.SCXML.parse(xml) - {:error, errors, _warnings} = SC.Validator.validate(document) + {:ok, document} = SCXML.parse(xml) + {:error, errors, _warnings} = Validator.validate(document) assert length(errors) == 1 assert hd(errors) =~ "cannot have both initial attribute and initial element" @@ -158,7 +160,7 @@ defmodule SC.InitialStatesTest do } document = %Document{states: [compound_state]} - {:error, errors, _warnings} = SC.Validator.validate(document) + {:error, errors, _warnings} = Validator.validate(document) assert length(errors) >= 1 assert Enum.any?(errors, &String.contains?(&1, "must contain exactly one transition")) @@ -173,14 +175,14 @@ defmodule SC.InitialStatesTest do %State{ id: "__initial_1__", type: :initial, - transitions: [%SC.Transition{target: "nonexistent"}] + transitions: [%Transition{target: "nonexistent"}] }, %State{id: "child1", type: :atomic, states: []} ] } document = %Document{states: [compound_state]} - {:error, errors, _warnings} = SC.Validator.validate(document) + {:error, errors, _warnings} = Validator.validate(document) assert length(errors) >= 1 assert Enum.any?(errors, &String.contains?(&1, "not a valid direct child")) @@ -198,7 +200,7 @@ defmodule SC.InitialStatesTest do } document = %Document{states: [compound_state]} - {:error, errors, _warnings} = SC.Validator.validate(document) + {:error, errors, _warnings} = Validator.validate(document) assert length(errors) >= 1 assert Enum.any?(errors, &String.contains?(&1, "cannot have multiple initial elements")) @@ -219,10 +221,10 @@ defmodule SC.InitialStatesTest do """ - {:ok, document} = Parser.SCXML.parse(xml) - {:ok, state_chart} = SC.Interpreter.initialize(document) + {:ok, document} = SCXML.parse(xml) + {:ok, state_chart} = Interpreter.initialize(document) - active_states = SC.Interpreter.active_states(state_chart) |> MapSet.to_list() + active_states = Interpreter.active_states(state_chart) |> MapSet.to_list() # Should enter child2 as specified by initial element, not child1 (first child) assert active_states == ["child2"] end @@ -239,10 +241,10 @@ defmodule SC.InitialStatesTest do """ - {:ok, document} = Parser.SCXML.parse(xml) - {:ok, state_chart} = SC.Interpreter.initialize(document) + {:ok, document} = SCXML.parse(xml) + {:ok, state_chart} = Interpreter.initialize(document) - active_states = SC.Interpreter.active_states(state_chart) |> MapSet.to_list() + active_states = Interpreter.active_states(state_chart) |> MapSet.to_list() # Should enter first non-initial child assert active_states == ["child1"] end diff --git a/test/sc/interpreter/compound_state_test.exs b/test/sc/interpreter/compound_state_test.exs index ff81d15..99b4373 100644 --- a/test/sc/interpreter/compound_state_test.exs +++ b/test/sc/interpreter/compound_state_test.exs @@ -1,7 +1,8 @@ defmodule SC.Interpreter.CompoundStateTest do use ExUnit.Case, async: true - alias SC.{Interpreter, Parser.SCXML} + alias SC.{Event, Interpreter, Parser} + alias SC.Parser.SCXML describe "compound state entry" do test "enters initial child state automatically" do @@ -107,7 +108,7 @@ defmodule SC.Interpreter.CompoundStateTest do assert MapSet.equal?(active_states, MapSet.new(["simple"])) # Transition to compound state - event = SC.Event.new("go") + event = Event.new("go") {:ok, new_state_chart} = Interpreter.send_event(state_chart, event) # Should automatically enter child1 (initial child of compound state) diff --git a/test/sc/parser/hierarchy_test.exs b/test/sc/parser/hierarchy_test.exs index 550868b..3a89f12 100644 --- a/test/sc/parser/hierarchy_test.exs +++ b/test/sc/parser/hierarchy_test.exs @@ -1,7 +1,8 @@ defmodule SC.Parser.HierarchyTest do use ExUnit.Case, async: true - alias SC.{Document, Parser.SCXML} + alias SC.{Configuration, Document, Parser, Validator} + alias SC.Parser.SCXML describe "parent and depth fields" do test "sets correct parent and depth for nested states" do @@ -57,13 +58,13 @@ defmodule SC.Parser.HierarchyTest do """ {:ok, raw_document} = SCXML.parse(xml) - {:ok, document, _warnings} = SC.Validator.validate(raw_document) + {:ok, document, _warnings} = Validator.validate(raw_document) # Create a configuration with the deepest state active - config = SC.Configuration.new(["grandchild"]) + config = Configuration.new(["grandchild"]) # Get all active states including ancestors - active_ancestors = SC.Configuration.active_ancestors(config, document) + active_ancestors = Configuration.active_ancestors(config, document) # Should include the active state plus all its ancestors expected = MapSet.new(["grandchild", "child", "parent"]) diff --git a/test/sc/parser/parallel_parsing_test.exs b/test/sc/parser/parallel_parsing_test.exs index 6caf135..20c6632 100644 --- a/test/sc/parser/parallel_parsing_test.exs +++ b/test/sc/parser/parallel_parsing_test.exs @@ -1,7 +1,8 @@ defmodule SC.Parser.ParallelParsingTest do use ExUnit.Case, async: true - alias SC.{Document, Parser.SCXML} + alias SC.{Document, Parser, Validator} + alias SC.Parser.SCXML describe "parallel state parsing" do test "parses simple parallel state" do @@ -43,7 +44,7 @@ defmodule SC.Parser.ParallelParsingTest do """ {:ok, document} = SCXML.parse(xml) - {:ok, validated_document, _warnings} = SC.Validator.validate(document) + {:ok, validated_document, _warnings} = Validator.validate(document) [parallel_state] = validated_document.states assert parallel_state.type == :parallel @@ -69,7 +70,7 @@ defmodule SC.Parser.ParallelParsingTest do """ {:ok, document} = SCXML.parse(xml) - {:ok, validated_document, _warnings} = SC.Validator.validate(document) + {:ok, validated_document, _warnings} = Validator.validate(document) [parallel_state] = validated_document.states assert parallel_state.type == :parallel diff --git a/test/sc/validator/final_state_test.exs b/test/sc/validator/final_state_test.exs index e3e8c28..3d8bf2b 100644 --- a/test/sc/validator/final_state_test.exs +++ b/test/sc/validator/final_state_test.exs @@ -1,7 +1,8 @@ defmodule SC.Validator.FinalStateTest do use ExUnit.Case - alias SC.{Document, Parser.SCXML} + alias SC.{Document, Parser, Validator} + alias SC.Parser.SCXML test "preserves final state type during validation" do xml = """ @@ -15,7 +16,7 @@ defmodule SC.Validator.FinalStateTest do """ {:ok, document} = SCXML.parse(xml) - {:ok, validated_document, _warnings} = SC.Validator.validate(document) + {:ok, validated_document, _warnings} = Validator.validate(document) # Find the final state in the validated document final_state = Enum.find(validated_document.states, &(&1.id == "final_state")) @@ -39,7 +40,7 @@ defmodule SC.Validator.FinalStateTest do """ {:ok, document} = SCXML.parse(xml) - {:ok, validated_document, _warnings} = SC.Validator.validate(document) + {:ok, validated_document, _warnings} = Validator.validate(document) # Find the compound state compound_state = Enum.find(validated_document.states, &(&1.id == "compound")) @@ -64,7 +65,7 @@ defmodule SC.Validator.FinalStateTest do """ {:ok, document} = SCXML.parse(xml) - {:ok, validated_document, _warnings} = SC.Validator.validate(document) + {:ok, validated_document, _warnings} = Validator.validate(document) # Find the parallel state parallel_state = Enum.find(validated_document.states, &(&1.id == "parallel_state")) @@ -91,7 +92,7 @@ defmodule SC.Validator.FinalStateTest do """ {:ok, document} = SCXML.parse(xml) - {:ok, validated_document, _warnings} = SC.Validator.validate(document) + {:ok, validated_document, _warnings} = Validator.validate(document) # Check both final states are preserved final1 = Enum.find(validated_document.states, &(&1.id == "final1")) diff --git a/test/sc/validator/state_types_test.exs b/test/sc/validator/state_types_test.exs index e209bdc..8415207 100644 --- a/test/sc/validator/state_types_test.exs +++ b/test/sc/validator/state_types_test.exs @@ -1,7 +1,8 @@ defmodule SC.Validator.StateTypesTest do use ExUnit.Case, async: true - alias SC.{Document, Parser.SCXML} + alias SC.{Document, Parser, Validator} + alias SC.Parser.SCXML describe "state type determination at parse time" do test "atomic state type determined at parse time" do @@ -48,7 +49,7 @@ defmodule SC.Validator.StateTypesTest do """ {:ok, document} = SCXML.parse(xml) - {:error, _errors, _warnings} = SC.Validator.validate(document) + {:error, _errors, _warnings} = Validator.validate(document) # State types are determined at parse time regardless of validity [state] = document.states diff --git a/test/support/sc_case.ex b/test/support/sc_case.ex index d6a70af..1e0caa2 100644 --- a/test/support/sc_case.ex +++ b/test/support/sc_case.ex @@ -11,6 +11,7 @@ defmodule SC.Case do use ExUnit.CaseTemplate, async: true + alias ExUnit.Assertions alias SC.{Event, FeatureDetector, Interpreter, Parser.SCXML} using do @@ -46,7 +47,7 @@ defmodule SC.Case do # Test uses unsupported features - fail with descriptive message unsupported_list = unsupported_features |> Enum.sort() |> Enum.join(", ") - ExUnit.Assertions.flunk(""" + Assertions.flunk(""" Test depends on unsupported SCXML features: #{unsupported_list} This test cannot pass until these features are implemented in the SC library. From f75c0b0eb573b433b3607356b8e83881367fed1e Mon Sep 17 00:00:00 2001 From: JohnnyT Date: Mon, 18 Aug 2025 09:26:47 -0600 Subject: [PATCH 3/8] Updates documentation --- CLAUDE.md | 25 +++++++++++++---- README.md | 82 +++++++++++++++++++++++++++++++++---------------------- 2 files changed, 70 insertions(+), 37 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9be6e47..71f827c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -84,12 +84,17 @@ Also use this initial Elixir implementation as reference: https://github.com/cam - **`SC.Parser.SCXML.StateStack`** - Manages parsing state stack for hierarchical document construction ### Validation and Optimization (Validate + Optimize Phases) -- **`SC.Document.Validator`** - Document validation and optimization +- **`SC.Validator`** - Main validation orchestrator (refactored from monolithic `SC.Document.Validator`) + - **Modular architecture**: Split into focused sub-validators for maintainability - **Validation**: Structural correctness, semantic consistency, reference validation - **Optimization**: Builds O(1) lookup maps via `finalize/2` for valid documents only - Returns `{:ok, optimized_document, warnings}` or `{:error, errors, warnings}` - **Clean architecture**: Only optimizes documents that pass validation - - Uses `find_state_by_id_linear/2` during validation, switches to O(1) after optimization +- **`SC.Validator.StateValidator`** - State ID uniqueness and validation +- **`SC.Validator.TransitionValidator`** - Transition target validation +- **`SC.Validator.InitialStateValidator`** - All initial state constraints (attributes, elements, conflicts) +- **`SC.Validator.ReachabilityAnalyzer`** - State reachability graph analysis +- **`SC.Validator.Utils`** - Shared utilities across validators ### Interpreter and Runtime - **`SC.Interpreter`** - Core SCXML interpreter with synchronous API @@ -119,7 +124,7 @@ The implementation follows a clean **Parse → Validate → Optimize** architect {:ok, document} = SC.Parser.SCXML.parse(xml_string) # 2. Validate + Optimize Phase: Check semantics + build lookup maps -{:ok, optimized_document, warnings} = SC.Document.Validator.validate(document) +{:ok, optimized_document, warnings} = SC.Validator.validate(document) # 3. Interpret Phase: Use optimized document for runtime {:ok, state_chart} = SC.Interpreter.initialize(optimized_document) @@ -131,7 +136,12 @@ The implementation follows a clean **Parse → Validate → Optimize** architect - Only valid documents get expensive optimization treatment - Clear separation of concerns across phases -### Test Infrastructure +### Feature Detection and Test Infrastructure +- **`SC.FeatureDetector`** - Detects SCXML features used in documents + - Enables proper test validation by failing tests that depend on unsupported features + - Prevents false positive test results from unsupported feature usage + - Supports both XML string and parsed document analysis + - Tracks feature support status (`:supported`, `:unsupported`, `:partial`) - **`SC.Case`** - Test case template module for SCXML testing - Provides `test_scxml/4` function for testing state machine behavior - Uses SC.Interpreter for document initialization and event processing @@ -217,11 +227,13 @@ XML content within triple quotes uses 4-space base indentation. **Working Features:** - ✅ Basic state transitions (basic1, basic2 tests pass) - ✅ **Compound states** with automatic initial child entry +- ✅ **Initial state elements** (`` with transitions) - W3C compliant - ✅ Hierarchical states with O(1) optimized lookups - ✅ Event-driven state changes -- ✅ Initial state configuration +- ✅ Initial state configuration (both `initial="id"` attributes and `` elements) - ✅ Document validation and error reporting - ✅ **Parse → Validate → Optimize** architecture +- ✅ **Modular validator architecture** with focused sub-validators **Main Failure Categories:** - **Document parsing failures**: Complex SCXML with parallel states, history states, executable content @@ -251,6 +263,9 @@ XML content within triple quotes uses 4-space base indentation. - Active state tracking with hierarchical ancestor computation using O(1) lookups - **Git pre-push hook** for automated local validation workflow - 95%+ test coverage maintained +- **Initial state elements** (`` with ``) with comprehensive validation +- **Modular validator architecture** - refactored from 386-line monolith into focused modules +- **Full Credo compliance** - all 43 alias-related issues resolved 🚧 **Future Extensions:** - Parallel states (``) - major gap in current implementation diff --git a/README.md b/README.md index 96c6218..b1d9a1f 100644 --- a/README.md +++ b/README.md @@ -9,15 +9,18 @@ An Elixir implementation of SCXML (State Chart XML) state charts with a focus on - ✅ **Complete SCXML Parser** - Converts XML documents to structured data with precise location tracking - ✅ **State Chart Interpreter** - Runtime engine for executing SCXML state charts -- ✅ **Comprehensive Validation** - Document validation with detailed error reporting +- ✅ **Modular Validation** - Document validation with focused sub-validators for maintainability - ✅ **Compound States** - Support for hierarchical states with automatic initial child entry +- ✅ **Initial State Elements** - Full support for `` elements with transitions (W3C compliant) - ✅ **Parallel States** - Support for concurrent state regions with simultaneous execution - ✅ **O(1) Performance** - Optimized state and transition lookups via Maps - ✅ **Event Processing** - Internal and external event queues per SCXML specification - ✅ **Parse → Validate → Optimize Architecture** - Clean separation of concerns +- ✅ **Feature Detection** - Automatic SCXML feature detection for test validation - ✅ **Regression Testing** - Automated tracking of passing tests to prevent regressions - ✅ **Git Hooks** - Pre-push validation workflow to catch issues early - ✅ **Test Infrastructure** - Compatible with SCION and W3C test suites +- ✅ **Code Quality** - Full Credo compliance with proper module aliasing ## Current Status @@ -28,11 +31,14 @@ An Elixir implementation of SCXML (State Chart XML) state charts with a focus on ### Working Features - ✅ **Basic state transitions** and event-driven changes - ✅ **Hierarchical states** with optimized O(1) state lookup and automatic initial child entry +- ✅ **Initial state elements** - Full `` element support with transitions and comprehensive validation - ✅ **Parallel states** with concurrent execution of multiple regions -- ✅ **Document validation** and error reporting with comprehensive structural checks +- ✅ **Modular validation** - Refactored from 386-line monolith into focused sub-validators +- ✅ **Feature detection** - Automatic SCXML feature detection prevents false positive test results - ✅ **SAX-based XML parsing** with accurate location tracking for error reporting - ✅ **Performance optimizations** - O(1) state/transition lookups, optimized active configuration - ✅ **Source field optimization** - Transitions include source state for faster event processing +- ✅ **Code quality** - Full Credo compliance with proper module aliasing throughout codebase ### Planned Features - History states (``) @@ -42,38 +48,49 @@ An Elixir implementation of SCXML (State Chart XML) state charts with a focus on - Expression evaluation and datamodel support - Enhanced validation for complex SCXML constructs -## Future Extensions +## Recent Completions -### **Feature-Based Test Validation System** -An enhancement to improve test accuracy by validating that tests actually exercise intended SCXML functionality: +### **✅ Feature-Based Test Validation System** +**COMPLETED** - Improves test accuracy by validating that tests actually exercise intended SCXML functionality: -**Goal**: Prevent false positive tests where unsupported features are silently ignored, leading to "passing" tests that don't actually validate the intended behavior. +- **`SC.FeatureDetector`** - Analyzes SCXML documents to detect used features +- **Feature validation** - Tests fail when they depend on unsupported features +- **False positive prevention** - No more "passing" tests that silently ignore unsupported features +- **Capability tracking** - Clear visibility into which SCXML features are supported -**Proposed Enhancement**: -```elixir -# Example test with feature requirements -defmodule SCIONTest.SendIdlocation.Test0Test do - use SC.Case - @tag :scion - @required_features [:datamodel, :send_elements, :onentry_actions, :conditional_transitions] - - test "test0" do - # Test implementation that requires these features - end -end -``` +### **✅ Modular Validator Architecture** +**COMPLETED** - Refactored monolithic validator into focused, maintainable modules: + +- **`SC.Validator`** - Main orchestrator (from 386-line monolith) +- **`SC.Validator.StateValidator`** - State ID validation +- **`SC.Validator.TransitionValidator`** - Transition target validation +- **`SC.Validator.InitialStateValidator`** - All initial state constraints +- **`SC.Validator.ReachabilityAnalyzer`** - State reachability analysis +- **`SC.Validator.Utils`** - Shared utilities + +### **✅ Initial State Elements** +**COMPLETED** - Full W3C-compliant support for `` elements: + +- **Parser support** - `` elements with `` children +- **Interpreter logic** - Proper initial state entry via initial elements +- **Comprehensive validation** - Conflict detection, target validation, structure validation +- **Feature detection** - Automatic detection of initial element usage + +## Future Extensions + +The next major areas for development focus on expanding SCXML feature support: -**Implementation Phases**: -1. **Feature Detection Phase** - Analyze SCXML documents to identify used features -2. **Feature Validation Phase** - Fail tests when required features are unsupported -3. **Test Annotation Phase** - Add `@required_features` tags to existing tests -4. **Incremental Implementation** - Enable feature flags as capabilities are added +### **High Priority Features** +- **Conditional Transitions** - `cond` attribute evaluation for dynamic transitions +- **Executable Content** - ``, ``, ``, `