Skip to content

[Validation] free function validation between puml and cpp - #518

Merged
hoe-jo merged 3 commits into
eclipse-score:mainfrom
melodyoncode:melody_validate_free_function
Oct 14, 2026
Merged

hoe-jo merged 3 commits into
eclipse-score:mainfrom
melodyoncode:melody_validate_free_function

Conversation

@melodyoncode

Copy link
Copy Markdown
Contributor

No description provided.

@melodyoncode melodyoncode changed the title Melody validate free function [Validation] free function validation between puml and cpp Oct 13, 2026
error.build()
}

fn format_missing_free_function(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

format_missing_free_function shows qualified_name() without the parameter list. With overloads Select(int) and Select(int, int) both missing

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update logic and the intergration test "negative_free_function_overloads_missing"

design_function: &FreeFunctionDecl,
implementation_function: &FreeFunctionDecl,
) {
let function_name = design_function.qualified_name();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reader stores enclosing_namespace_id with . separators (uid_utils::normalize), and qualified_name() then adds ::. Nested namespaces therefore show as vehicle.core::Dispatch, while the suggestion line shows vehicle::core::Dispatch()
(same for L 145, L605, L635)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out. I normalized free-function qualified names to use :: consistently in all affected diagnostic locations, including the ones mentioned above. I also added a regression test for nested namespaces to ensure diagnostics consistently render vehicle::core::Dispatch.

.join(", ")
}

fn parameter_differences(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

method error output changed. method "X" parameter list is now split into parameter variadic and parameter pack expansion, so one parameter can produce two errors that show the same full parameter list.

Any Suggestion how we can solve it differently? If too complicated, then lets keep it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the suggestion. I considered rendering only the affected parameter for these diagnostics.

I kept the full parameter list because it provides more useful context for variadic and pack-expansion mismatches. Rendering only the affected parameter can also lose information; for example, a C-style variadic parameter may be rendered as <unknown>.

For example:

// Puml file:
  Dispatch(...)

// Cpp Implementation:
  Dispatch(int argument)

// Diagnostic values:
  Design value         : (...)
  Implementation value : (argument: int)

Similarly, showing the complete signatures makes a pack-expansion mismatch clear:

// Puml file:
  Expand(id : int, , retry : bool, argument : int)

// Cpp Implementation:
  Template <typename... Arguments>
  Expand(int id, bool retry, Arguments... arguments)

// Diagnostic values: 
  Design value         : (id : int, , retry : bool, argument : int)
  Implementation value : (int id, bool retry, Arguments... arguments)

Also, C-style variadic parameters (...) and template pack expansions (T... args) are distinct C++ constructs, so the same parameter should not produce both errors. Therefore, we do not expect duplicated full-list output for a single parameter.

I updated the integration coverage to keep the complete parameter-list output for both cases.

}
}

free_functions.extend(diagram.free_functions.iter().cloned());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

free functions are appended without checking for duplicates, while duplicate classes raise "defined more than once"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for raising this. I considered duplicate handling for free functions while working on this change.

However, free functions support overloads, and the C++ parser already filters duplicate declarations. Adding validation here would therefore require a separate, signature-based definition of a duplicate and corresponding coverage for its edge cases. Given that scope and the limited benefit for the current change, I decided not to introduce it here.

int Select(int mode, int force);

template <typename Payload>
Payload Convert(Payload payload){}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Payload Convert(Payload payload){} is a non-void function with an empty body -> undefined behaviour

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update the test example:)

@melodyoncode
melodyoncode force-pushed the melody_validate_free_function branch from ec1928a to 70cb47b Compare October 14, 2026 08:30
@hoe-jo
hoe-jo added this pull request to the merge queue Oct 14, 2026
Merged via the queue into eclipse-score:main with commit 6389c2a Oct 14, 2026
12 checks passed
@melodyoncode
melodyoncode deleted the melody_validate_free_function branch October 15, 2026 01:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants