log: `mw_log_fmt_macro` implementation by arkjedrz · Pull Request #14 · eclipse-score/baselibs_rust

arkjedrz · 2025-12-01T09:50:32Z

Replacement for format_args! macro.
Replacement for Debug derive macro.
Unit tests.

Notes for Reviewer

Pre-Review Checklist for the PR Author

PR title is short, expressive and meaningful
Commits are properly organized
Relevant issues are linked in the References section
Tests are conducted
Unit tests are added

Checklist for the PR Reviewer

Commits are properly organized and messages are according to the guideline
Unit tests have been written for new behavior
Public API is documented
PR title describes the changes

Post-review Checklist for the PR Author

All open points are addressed and tracked via issues

References

Closes #24

github-actions · 2025-12-01T09:51:15Z

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run //:license-check

Status: ⚠️ Needs Review

Click to expand output

[License Check Output]
Extracting Bazel installation...
Starting local Bazel server (8.4.2) and connecting to it...
INFO: Invocation ID: a5202e0d-35ca-4457-87bf-ec18349d877b
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
DEBUG: Rule 'rust_qnx8_toolchain+' indicated that a canonical reproducible form can be obtained by modifying arguments integrity = "sha256-eQOopREOYCL5vtTb6c1cwZrql4GVrJ1FqgxarQRe1xs="
DEBUG: Repository rust_qnx8_toolchain+ instantiated at:
  <builtin>: in <toplevel>
Repository rule http_archive defined at:
  /home/runner/.bazel/external/bazel_tools/tools/build_defs/repo/http.bzl:431:31: in <toplevel>
Computing main repo mapping: 
Computing main repo mapping: 
WARNING: For repository 'bazel_skylib', the root module requires module version bazel_skylib@1.7.1, but got bazel_skylib@1.8.1 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: For repository 'rules_cc', the root module requires module version rules_cc@0.1.1, but got rules_cc@0.1.2 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: For repository 'aspect_rules_lint', the root module requires module version aspect_rules_lint@1.0.3, but got aspect_rules_lint@1.5.3 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: For repository 'buildifier_prebuilt', the root module requires module version buildifier_prebuilt@7.3.1, but got buildifier_prebuilt@8.2.0.2 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: For repository 'score_docs_as_code', the root module requires module version score_docs_as_code@2.0.2, but got score_docs_as_code@2.2.0 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
Loading: 
Loading: 0 packages loaded
Loading: 0 packages loaded
Loading: 0 packages loaded
    currently loading: 
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)

Analyzing: target //:license-check (31 packages loaded, 10 targets configured)

Analyzing: target //:license-check (85 packages loaded, 10 targets configured)

Analyzing: target //:license-check (143 packages loaded, 1771 targets configured)

Analyzing: target //:license-check (145 packages loaded, 2719 targets configured)

Analyzing: target //:license-check (145 packages loaded, 2719 targets configured)

Analyzing: target //:license-check (145 packages loaded, 2719 targets configured)

Analyzing: target //:license-check (156 packages loaded, 8996 targets configured)

INFO: Analyzed target //:license-check (156 packages loaded, 9013 targets configured).
INFO: From Generating Dash formatted dependency file ...:
INFO: Successfully converted 4 packages from Cargo.lock to bazel-out/k8-fastbuild/bin/formatted.txt
INFO: Found 1 target...
Target //:license.check.license_check up-to-date:
  bazel-bin/license.check.license_check
  bazel-bin/license.check.license_check.jar
INFO: Elapsed time: 23.414s, Critical Path: 0.41s
INFO: 14 processes: 5 disk cache hit, 9 internal.
INFO: Build completed successfully, 14 total actions
INFO: Running command line: bazel-bin/license.check.license_check ./formatted.txt <args omitted>
usage: org.eclipse.dash.licenses.cli.Main [-batch <int>] [-cd <url>]
       [-confidence <int>] [-ef <url>] [-excludeSources <sources>] [-help] [-lic
       <url>] [-project <shortname>] [-repo <url>] [-review] [-summary <file>]
       [-timeout <seconds>] [-token <token>]

github-actions · 2025-12-01T09:52:40Z

The created documentation from the pull request is available at: docu-html

Copilot

Pull request overview

This PR implements the mw_log_fmt_macro crate, providing a custom replacement for Rust's format_args! macro. The implementation includes comprehensive unit tests and removes placeholder code from the log library.

Key changes:

Implements format string parsing with support for placeholders, format specifications, and argument handling
Adds comprehensive test coverage for macro functionality and format specifications
Updates documentation to reflect the correct crate naming (mw_log_fmt_macro)

Reviewed changes

Copilot reviewed 25 out of 26 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/log/mw_log_fmt_macro/format_args.rs	Core macro implementation with format string parsing and code generation logic
src/log/mw_log_fmt_macro/lib.rs	Public proc macro interface exposing `mw_log_format_args!` and `mw_log_format_args_nl!`
src/log/mw_log_fmt_macro/tests/test_format_args.rs	Comprehensive unit tests for macro functionality
src/log/mw_log_fmt/*.rs	Supporting formatting library files with traits, builders, and implementations
docs/*/.puml	Documentation updates to use correct crate name `mw_log_fmt_macro`
src/log/src/lib.rs	Removed placeholder code
Cargo.toml	Updated workspace configuration with new crates

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/log/mw_log_fmt/fmt_spec.rs

src/log/mw_log_fmt_macro/format_args.rs

Copilot

Pull request overview

Copilot reviewed 25 out of 26 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-09T11:58:39Z

src/log/mw_log_fmt_macro/format_args.rs

+    for arg in args.iter() {
+        let (arg_expr, alias_expr) = match arg {
+            Expr::Assign(expr_assign) => (expr_assign.left.as_ref().clone(), Some(expr_assign.right.as_ref().clone())),
+            Expr::Lit(_) | Expr::Field(_) | Expr::If(_) | Expr::Path(_) | Expr::Unary(_) => (arg.clone(), None),


The same expression type matching pattern appears on lines 466 and 519. Consider extracting this into a helper function or constant to reduce duplication and ensure consistency if the list of allowed types needs to be updated.

Copilot · 2025-12-09T11:58:40Z

src/log/mw_log_fmt/fmt_impl_qm.rs

+use crate::fmt_spec::FormatSpec;
+use std::path::{Path, PathBuf};
+
+// TODO: replace with `core::char::MAX_LEN_UTF8` once stable.


Consider adding a tracking link or issue number to the TODO comment so maintainers can easily determine when core::char::MAX_LEN_UTF8 becomes stable and this workaround can be removed.

Suggested change

// TODO: replace with `core::char::MAX_LEN_UTF8` once stable.

// TODO: replace with `core::char::MAX_LEN_UTF8` once stable. Tracking: https://github.com/rust-lang/rust/issues/56345

awillenbuecher-xq-tec

The code base seems to be wrongly formatted, cargo fmt --check shows a lot of differences for me.

src/log/mw_log_fmt_macro/tests/test_format_args.rs

src/log/mw_log_fmt_macro/format_args.rs

arkjedrz · 2025-12-12T07:51:33Z

@awillenbuecher-xq-tec
Thanks for the review!
Regarding formatting - there's a mismatch in formatting config between Bazel and Cargo. Reported: #33

pawelrutkaq

overall q: do we need to add tests for macro code generators using some specilzied crate for that ?

src/log/mw_log_fmt_macro/score_debug.rs

src/log/mw_log_fmt_macro/format_args.rs

arkjedrz · 2025-12-18T07:46:07Z

@pawelrutkaq
Regarding testing proc-macros - was the path on how to do it investigated somewhere in the S-CORE?
I can create a ticket for that and handle that separately, since this package is tested as much built-in Rust tooling offers.

awillenbuecher-xq-tec · 2025-12-18T07:52:59Z

@pawelrutkaq @arkjedrz It might make sense to test the proc-macro directly by checking that the generated code (or more precisely, the generated token stream) exactly matches an expected code/token stream. However, I would only add these tests once the implementation is pretty much finished, because any change in the proc-macro would require wide changes in the tests.

pawelrutkaq · 2025-12-18T08:58:20Z

Lets create ticket for proc-macoe testing as improvmenets later for now.

arkjedrz · 2025-12-18T09:29:42Z

Ticket for tests for proc-macro: #43

- Replacement for `format_args!` macro. - Replacement for `Debug` derive macro. - Unit tests.

arkjedrz force-pushed the arkjedrz_mw-log-macro branch 7 times, most recently from d6c0599 to 8992014 Compare December 9, 2025 10:42

arkjedrz changed the title ~~log: mw_log_macro implementation~~ log: mw_log_fmt_macro implementation Dec 9, 2025

arkjedrz marked this pull request as ready for review December 9, 2025 10:50

arkjedrz requested review from awillenbuecher-xq-tec, Copilot and pawelrutkaq December 9, 2025 10:50

Copilot AI reviewed Dec 9, 2025

View reviewed changes

src/log/mw_log_fmt/fmt_spec.rs Outdated Show resolved Hide resolved

src/log/mw_log_fmt/fmt_spec.rs Outdated Show resolved Hide resolved

src/log/mw_log_fmt_macro/format_args.rs Outdated Show resolved Hide resolved

arkjedrz force-pushed the arkjedrz_mw-log-macro branch from 8992014 to c65d6e3 Compare December 9, 2025 11:56

arkjedrz requested a review from Copilot December 9, 2025 11:57

Copilot AI reviewed Dec 9, 2025

View reviewed changes

arkjedrz force-pushed the arkjedrz_mw-log-macro branch 8 times, most recently from 0ad8eea to 9d1a8c6 Compare December 11, 2025 11:12

awillenbuecher-xq-tec suggested changes Dec 11, 2025

View reviewed changes

arkjedrz force-pushed the arkjedrz_mw-log-macro branch 2 times, most recently from 25ab826 to b650ed5 Compare December 12, 2025 17:21

arkjedrz requested a review from awillenbuecher-xq-tec December 12, 2025 17:22

arkjedrz force-pushed the arkjedrz_mw-log-macro branch from b650ed5 to 80834c8 Compare December 15, 2025 07:17

awillenbuecher-xq-tec approved these changes Dec 15, 2025

View reviewed changes

arkjedrz mentioned this pull request Dec 15, 2025

Improvement: Add ScoreDebug macros #24

Closed

arkjedrz force-pushed the arkjedrz_mw-log-macro branch 2 times, most recently from 2bf9080 to 7dd13ef Compare December 16, 2025 13:18

arkjedrz self-assigned this Dec 16, 2025

arkjedrz force-pushed the arkjedrz_mw-log-macro branch from 7dd13ef to ee5f163 Compare December 17, 2025 12:05

pawelrutkaq requested changes Dec 17, 2025

View reviewed changes

src/log/mw_log_fmt_macro/score_debug.rs Show resolved Hide resolved

src/log/mw_log_fmt_macro/format_args.rs Outdated Show resolved Hide resolved

arkjedrz force-pushed the arkjedrz_mw-log-macro branch from ee5f163 to f4494ac Compare December 18, 2025 09:27

arkjedrz had a problem deploying to workflow-approval December 18, 2025 09:27 — with GitHub Actions Failure

arkjedrz requested a review from pawelrutkaq December 18, 2025 09:32

arkjedrz force-pushed the arkjedrz_mw-log-macro branch from f4494ac to e250c50 Compare December 18, 2025 09:37

arkjedrz had a problem deploying to workflow-approval December 18, 2025 09:37 — with GitHub Actions Failure

log: mw_log_fmt_macro implementation

a9b44fa

- Replacement for `format_args!` macro. - Replacement for `Debug` derive macro. - Unit tests.

arkjedrz force-pushed the arkjedrz_mw-log-macro branch from e250c50 to a9b44fa Compare December 18, 2025 10:03

arkjedrz temporarily deployed to workflow-approval December 18, 2025 10:03 — with GitHub Actions Inactive

pawelrutkaq approved these changes Dec 18, 2025

View reviewed changes

pawelrutkaq merged commit b283d54 into eclipse-score:main Dec 18, 2025
12 checks passed

arkjedrz deleted the arkjedrz_mw-log-macro branch December 18, 2025 10:14

	// TODO: replace with `core::char::MAX_LEN_UTF8` once stable.
	// TODO: replace with `core::char::MAX_LEN_UTF8` once stable. Tracking: https://github.com/rust-lang/rust/issues/56345

Comments

Conversation

arkjedrz commented Dec 1, 2025 • edited by pawelrutkaq Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Notes for Reviewer

Pre-Review Checklist for the PR Author

Checklist for the PR Reviewer

Post-review Checklist for the PR Author

References

Uh oh!

github-actions bot commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

License Check Results

Uh oh!

github-actions bot commented Dec 1, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

awillenbuecher-xq-tec left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

arkjedrz commented Dec 12, 2025

Uh oh!

pawelrutkaq left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

arkjedrz commented Dec 18, 2025

Uh oh!

awillenbuecher-xq-tec commented Dec 18, 2025

Uh oh!

pawelrutkaq commented Dec 18, 2025

Uh oh!

arkjedrz commented Dec 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

arkjedrz commented Dec 1, 2025 •

edited by pawelrutkaq

Loading

github-actions bot commented Dec 1, 2025 •

edited

Loading