Add trait bounds to reject invalid Writers pipelines (#162)

- rename `Normalized` and `Summarized` `Writer`s to `Normalize` and `Summarize`
- add `Normalized` trait and require it for `Writer`s in `Cucumber` run methods
- add `NonTransforming` trait and require it for `writer::Repeat`
- add `Summarizable` trait and require it for `writer::Summarize`

Co-authored-by: Kai Ren <[email protected]>

CHANGELOG.md

@@ -14,6 +14,8 @@ All user visible changes to `cucumber` crate will be documented in this file. Th
 ### BC Breaks
 
 - Moved `World` type parameter of `WriterExt` trait to methods. ([#160])
+- Renamed `Normalized` and `Summarized` `Writer`s to `Normalize` and `Summarize`. ([#162])
+- Removed `writer::Basic` `Default` impl and change `writer::Basic::new()` return type to `writer::Normalize<writer::Basic>`. ([#162])
 
 ### Added
 
@@ -23,6 +25,10 @@ All user visible changes to `cucumber` crate will be documented in this file. Th
 - `writer::Json` ([Cucumber JSON format][0110-2]) behind the `output-json` feature flag. ([#159])
 - `writer::Tee` for outputting to multiple terminating `Writer`s simultaneously. ([#160])
 - `writer::discard::Arbitrary` and `writer::discard::Failure` for providing no-op implementations of the corresponding `Writer` traits. ([#160])
+- Inability to build invalid `Writer`s pipelines:
+ - `writer::Normalized` trait required for `Writer`s in `Cucumber` running methods. ([#162])
+ - `writer::NonTransforming` trait required for `writer::Repeat`. ([#162])
+ - `writer::Summarizable` trait required for `writer::Summarize`. ([#162])
 
 ### Fixed
 
@@ -32,6 +38,7 @@ All user visible changes to `cucumber` crate will be documented in this file. Th
 [#151]: /../../pull/151
 [#159]: /../../pull/159
 [#160]: /../../pull/160
+[#162]: /../../pull/162
 [#163]: /../../pull/163
 [0110-1]: https://llg.cubic.org/docs/junit
 [0110-2]: https://github.com/cucumber/cucumber-json-schema

book/src/Features.md

@@ -508,10 +508,11 @@ use cucumber::{writer, WriterExt as _};
 let file = fs::File::create(dbg!(format!("{}/target/schema.json", env!("CARGO_MANIFEST_DIR"))))?;
 World::cucumber()
  .with_writer(
- writer::Basic::default()
- .summarized()
- .tee::<World, _>(writer::Json::for_tee(file))
- .normalized(),
+ // `Writer`s pipeline is constructed in a reversed order.
+ writer::Basic::stdout() // And output to STDOUT.
+ .summarized() // Simultaneously, add execution summary.
+ .tee::<World, _>(writer::Json::for_tee(file)) // Then, output to JSON file.
+ .normalized() // First, normalize events order.
  )
  .run_and_exit("tests/features/book")
  .await;

src/cli.rs

@@ -173,9 +173,7 @@ This struct is especially useful, when implementing custom [`Writer`] wrapping
 another one:
 ```rust
 # use async_trait::async_trait;
-# use cucumber::{
-# cli, event, parser, ArbitraryWriter, Event, FailureWriter, World, Writer,
-# };
+# use cucumber::{cli, event, parser, writer, Event, World, Writer};
 # use structopt::StructOpt;
 #
 struct CustomWriter<Wr>(Wr);
@@ -208,11 +206,11 @@ where
 // Useful blanket impls:
 
 #[async_trait(?Send)]
-impl<'val, W, Wr, Val> ArbitraryWriter<'val, W, Val> for CustomWriter<Wr>
+impl<'val, W, Wr, Val> writer::Arbitrary<'val, W, Val> for CustomWriter<Wr>
 where
  W: World,
  Self: Writer<W>,
- Wr: ArbitraryWriter<'val, W, Val>,
+ Wr: writer::Arbitrary<'val, W, Val>,
  Val: 'val,
 {
  async fn write(&mut self, val: Val)
@@ -223,11 +221,11 @@ where
  }
 }
 
-impl<W, Wr> FailureWriter<W> for CustomWriter<Wr>
+impl<W, Wr> writer::Failure<W> for CustomWriter<Wr>
 where
  W: World,
  Self: Writer<W>,
- Wr: FailureWriter<W>,
+ Wr: writer::Failure<W>,
 {
  fn failed_steps(&self) -> usize {
  self.0.failed_steps()
@@ -241,6 +239,12 @@ where
  self.0.hook_errors()
  }
 }
+
+impl<Wr: writer::Normalized> writer::Normalized for CustomWriter<Wr> {}
+
+impl<Wr: writer::NonTransforming> writer::NonTransforming
+ for CustomWriter<Wr>
+{}
 ```
 
 [`Writer`]: crate::Writer

src/cucumber.rs

@@ -25,9 +25,8 @@ use regex::Regex;
 use structopt::{StructOpt, StructOptInternal};
 
 use crate::{
- cli, event, parser, runner, step, tag::Ext as _, writer, Event,
- FailureWriter, Parser, Runner, ScenarioType, Step, World, Writer,
- WriterExt as _,
+ cli, event, parser, runner, step, tag::Ext as _, writer, Event, Parser,
+ Runner, ScenarioType, Step, World, Writer, WriterExt as _,
 };
 
 /// Top-level [Cucumber] executor.
@@ -214,7 +213,10 @@ where
  #[must_use]
  pub fn repeat_skipped(
  self,
- ) -> Cucumber<W, P, I, R, writer::Repeat<W, Wr>, Cli> {
+ ) -> Cucumber<W, P, I, R, writer::Repeat<W, Wr>, Cli>
+ where
+ Wr: writer::NonTransforming,
+ {
  Cucumber {
  parser: self.parser,
  runner: self.runner,
@@ -296,20 +298,14 @@ where
  /// async data-autoplay="true" data-rows="24">
  /// </script>
  ///
- /// > ⚠️ __WARNING__: [`Cucumber::repeat_failed()`] should be called before
- /// [`Cucumber::fail_on_skipped()`], as events pass from
- /// outer [`Writer`]s to inner ones. So we need to
- /// transform [`Skipped`] to [`Failed`] first, and only
- /// then [`Repeat`] them.
- ///
  /// [`Failed`]: crate::event::Step::Failed
- /// [`Repeat`]: writer::Repeat
- /// [`Scenario`]: gherkin::Scenario
- /// [`Skipped`]: crate::event::Step::Skipped
  #[must_use]
  pub fn repeat_failed(
  self,
- ) -> Cucumber<W, P, I, R, writer::Repeat<W, Wr>, Cli> {
+ ) -> Cucumber<W, P, I, R, writer::Repeat<W, Wr>, Cli>
+ where
+ Wr: writer::NonTransforming,
+ {
  Cucumber {
  parser: self.parser,
  runner: self.runner,
@@ -414,23 +410,15 @@ where
  /// async data-autoplay="true" data-rows="24">
  /// </script>
  ///
- /// > ⚠️ __WARNING__: [`Cucumber::repeat_if()`] should be called before
- /// [`Cucumber::fail_on_skipped()`], as events pass from
- /// outer [`Writer`]s to inner ones. So we need to
- /// transform [`Skipped`] to [`Failed`] first, and only
- /// then [`Repeat`] them.
- ///
  /// [`Failed`]: crate::event::Step::Failed
- /// [`Repeat`]: writer::Repeat
- /// [`Scenario`]: gherkin::Scenario
- /// [`Skipped`]: crate::event::Step::Skipped
  #[must_use]
  pub fn repeat_if<F>(
  self,
  filter: F,
  ) -> Cucumber<W, P, I, R, writer::Repeat<W, Wr, F>, Cli>
  where
  F: Fn(&parser::Result<Event<event::Cucumber<W>>>) -> bool,
+ Wr: writer::NonTransforming,
  {
  Cucumber {
  parser: self.parser,
@@ -638,7 +626,7 @@ where
  W: World,
  P: Parser<I>,
  R: Runner<W>,
- Wr: Writer<W>,
+ Wr: Writer<W> + writer::Normalized,
  Cli: StructOpt + StructOptInternal,
 {
  /// Runs [`Cucumber`].
@@ -900,7 +888,7 @@ pub(crate) type DefaultCucumber<W, I> = Cucumber<
  parser::Basic,
  I,
  runner::Basic<W>,
- writer::Summarized<writer::Normalized<W, writer::Basic>>,
+ writer::Summarize<writer::Normalize<W, writer::Basic>>,
 >;
 
 impl<W, I> Default for DefaultCucumber<W, I>
@@ -912,7 +900,7 @@ where
  Self::custom(
  parser::Basic::new(),
  runner::Basic::default(),
- writer::Basic::default().normalized().summarized(),
+ writer::Basic::stdout().summarized(),
  )
  }
 }
@@ -931,15 +919,15 @@ where
  /// `@serial` [tag] is present on a [`Scenario`];
  /// * Allowed to run up to 64 [`Concurrent`] [`Scenario`]s.
  ///
- /// * [`Writer`] — [`Normalized`] and [`Summarized`] [`writer::Basic`].
+ /// * [`Writer`] — [`Normalize`] and [`Summarize`] [`writer::Basic`].
  ///
  /// [`Concurrent`]: runner::basic::ScenarioType::Concurrent
- /// [`Normalized`]: writer::Normalized
+ /// [`Normalize`]: writer::Normalize
  /// [`Parser`]: parser::Parser
  /// [`Scenario`]: gherkin::Scenario
  /// [`Serial`]: runner::basic::ScenarioType::Serial
  /// [`ScenarioType`]: runner::basic::ScenarioType
- /// [`Summarized`]: writer::Summarized
+ /// [`Summarize`]: writer::Summarize
  ///
  /// [tag]: https://cucumber.io/docs/cucumber/api/#tags
  #[must_use]
@@ -1174,7 +1162,7 @@ where
  W: World,
  P: Parser<I>,
  R: Runner<W>,
- Wr: FailureWriter<W>,
+ Wr: writer::Failure<W> + writer::Normalized,
  Cli: StructOpt + StructOptInternal,
 {
  /// Runs [`Cucumber`].

src/runner/mod.rs

@@ -40,23 +40,20 @@ pub use self::basic::{Basic, ScenarioType};
 /// some [`Scenario`], it should be resolved as [`ScenarioType::Serial`] by the
 /// [`Runner`].
 ///
+/// Because of that, [`Writer`], accepting events produced by a [`Runner`] has
+/// to be [`Normalized`].
+///
 /// All those rules are considered in a [`Basic`] reference [`Runner`]
 /// implementation.
 ///
-/// Note, that those rules are recommended in case you are using a
-/// [`writer::Normalized`]. Strictly speaking, no one is stopping you from
-/// implementing [`Runner`] which sources events completely out-of-order or even
-/// skips some of them. For example, this can be useful if you care only about
-/// failed [`Step`]s.
-///
 /// [`Cucumber`]: event::Cucumber
 /// [`Feature`]: gherkin::Feature
+/// [`Normalized`]: crate::writer::Normalized
 /// [`Parser`]: crate::Parser
 /// [`Rule`]: gherkin::Rule
 /// [`Scenario`]: gherkin::Scenario
 /// [`Step`]: gherkin::Step
 /// [`Writer`]: crate::Writer
-/// [`writer::Normalized`]: crate::writer::Normalized
 ///
 /// [happened-before]: https://en.wikipedia.org/wiki/Happened-before
 pub trait Runner<World> {

src/writer/basic.rs

@@ -27,8 +27,12 @@ use structopt::StructOpt;
 use crate::{
  event::{self, Info},
  parser,
- writer::out::{Styles, WriteStrExt as _},
- ArbitraryWriter, Event, World, Writer,
+ writer::{
+ self,
+ out::{Styles, WriteStrExt as _},
+ Ext as _,
+ },
+ Event, World, Writer,
 };
 
 // Workaround for overwritten doc-comments.
@@ -85,14 +89,13 @@ impl FromStr for Coloring {
 ///
 /// # Ordering
 ///
-/// It naively and immediately outputs anything it receives from a [`Runner`],
-/// so in case the later executes [`Scenario`]s concurrently, the output will be
-/// mixed and unordered. To output in a readable order, consider to wrap this
-/// [`Basic`] [`Writer`] into a [`writer::Normalized`].
+/// This [`Writer`] isn't [`Normalized`] by itself, so should be wrapped into
+/// a [`writer::Normalize`], otherwise will produce output [`Event`]s in a
+/// broken order.
 ///
+/// [`Normalized`]: writer::Normalized
 /// [`Runner`]: crate::runner::Runner
 /// [`Scenario`]: gherkin::Scenario
-/// [`writer::Normalized`]: crate::writer::Normalized
 #[derive(Debug, Deref, DerefMut)]
 pub struct Basic<Out: io::Write = io::Stdout> {
  /// [`io::Write`] implementor to write the output into.
@@ -149,7 +152,7 @@ where
 }
 
 #[async_trait(?Send)]
-impl<'val, W, Val, Out> ArbitraryWriter<'val, W, Val> for Basic<Out>
+impl<'val, W, Val, Out> writer::Arbitrary<'val, W, Val> for Basic<Out>
 where
  W: World + Debug,
  Val: AsRef<str> + 'val,
@@ -165,16 +168,43 @@ where
  }
 }
 
-impl Default for Basic {
- fn default() -> Self {
+impl<O: io::Write> writer::NonTransforming for Basic<O> {}
+
+impl Basic {
+ /// Creates a new [`Normalized`] [`Basic`] [`Writer`] outputting to
+ /// [`io::Stdout`].
+ ///
+ /// [`Normalized`]: writer::Normalized
+ #[must_use]
+ pub fn stdout<W>() -> writer::Normalize<W, Self> {
  Self::new(io::stdout(), Coloring::Auto, false)
  }
 }
 
 impl<Out: io::Write> Basic<Out> {
- /// Creates a new [`Basic`] [`Writer`].
+ /// Creates a new [`Normalized`] [`Basic`] [`Writer`] outputting to the
+ /// given `output`.
+ ///
+ /// [`Normalized`]: writer::Normalized
+ #[must_use]
+ pub fn new<W>(
+ output: Out,
+ color: Coloring,
+ verbose: bool,
+ ) -> writer::Normalize<W, Self> {
+ Self::raw(output, color, verbose).normalized()
+ }
+
+ /// Creates a new non-[`Normalized`] [`Basic`] [`Writer`] outputting to the
+ /// given `output`.
+ ///
+ /// Use it only if you know what you're doing. Otherwise, consider using
+ /// [`Basic::new()`] which creates an already [`Normalized`] version of a
+ /// [`Basic`] [`Writer`].
+ ///
+ /// [`Normalized`]: writer::Normalized
  #[must_use]
- pub fn new(output: Out, color: Coloring, verbose: bool) -> Self {
+ pub fn raw(output: Out, color: Coloring, verbose: bool) -> Self {
  let mut basic = Self {
  output,
  styles: Styles::new(),