Move some apollo-composition types into apollo-federation-types (#…

…611)

The current dependency tree ends up requiring that Rover pull in
`apollo-federation` even though it never needs that code. This has
resulted in a bunch of maintenance burden in trying to pin the correct
combination of dependencies.

This PR moves all the types that `apollo-language-server` depends on
into `apollo-federation-types` (which Rover already includes anyway).
The result is that `apollo-language-server` no longer requires
`apollo-composition`, so neither does Rover, and therefore neither pull
in `apollo-federation`.

I also added to the changelog that we should be pinning
`apollo-federation` as a reminder that when we do our next release we
should use `=2.0.0-preview.4` to avoid confusing churn in the packages
that _do_ still depend on `apollo-composition`.

<!-- CNN-527 -->

.circleci/config.yml

@@ -75,7 +75,7 @@ jobs:
           command: cargo fmt --all -- --check
       - run:
           name: Check Rust lints
-          command: cargo clippy -- -D warnings
+          command: cargo clippy --all-features -- -D warnings
 
   test:
     parameters:

Cargo.toml

@@ -1,3 +1,6 @@
 [workspace]
 members = ["apollo-federation-types", "apollo-composition"]
 resolver = "2"
+
+[workspace.dependencies]
+apollo-compiler = "1.0.0-beta.24"

apollo-composition/CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## 0.2.0 - Unreleased
+
+- Pin `apollo-federation` to 2.0.0-preview.4 to prevent future breaking changes
+- Move `Issue`, `Severity`, and `SubgraphLocation` to new `apollo_federation_types::composition` module so some 
+  consumers can avoid pulling in extra dependencies. Requires `apollo_federation_types` 
+
 ## 0.1.6
 - Update to `apollo-federation` 2.0.0-preview.3
 

apollo-composition/Cargo.toml

@@ -9,7 +9,7 @@ readme = "README.md"
 repository = "https://github.com/apollographql/federation-rs/"
 
 [dependencies]
-apollo-compiler = "1.0.0-beta.24"
+apollo-compiler = { workspace = true }
 apollo-federation = { version = "2.0.0-preview.3", git = "https://github.com/apollographql/router.git", branch = "next" }
-apollo-federation-types = { version = "0.15.0", path = "../apollo-federation-types" }
+apollo-federation-types = { version = "0.15.0", path = "../apollo-federation-types", features = ["composition"] }
 either = "1.12.0"

apollo-composition/src/lib.rs

@@ -1,20 +1,17 @@
-use apollo_compiler::parser::LineColumn;
+use std::collections::HashMap;
+use std::iter::once;
+
 use apollo_compiler::schema::ExtendedType;
-use apollo_federation::sources::connect::expand::{expand_connectors, Connectors, ExpansionResult};
-use apollo_federation::sources::connect::validation::{
-    validate, Severity as ValidationSeverity, ValidationResult,
-};
-use apollo_federation_types::build_plugin::{
-    BuildMessage, BuildMessageLevel, BuildMessageLocation, BuildMessagePoint,
+use apollo_federation::sources::connect::{
+    expand::{expand_connectors, Connectors, ExpansionResult},
+    validation::{validate, Severity as ValidationSeverity, ValidationResult},
 };
-use apollo_federation_types::javascript::{
-    CompositionHint, GraphQLError, SatisfiabilityResult, SubgraphASTNode, SubgraphDefinition,
+use apollo_federation_types::composition::SubgraphLocation;
+use apollo_federation_types::{
+    composition::{Issue, Severity},
+    javascript::{SatisfiabilityResult, SubgraphDefinition},
 };
-use apollo_federation_types::rover::{BuildError, BuildHint};
 use either::Either;
-use std::collections::HashMap;
-use std::iter::once;
-use std::ops::Range;
 
 /// This trait includes all the Rust-side composition logic, plus hooks for the JavaScript side.
 /// If you implement the functions in this trait to build your own JavaScript interface, then you
@@ -96,7 +93,7 @@ pub trait HybridComposition {
                                 range: Some(range),
                             })
                             .collect(),
-                        severity: validation_error.code.severity().into(),
+                        severity: convert_severity(validation_error.code.severity()),
                     })
             })
             .collect::<Vec<_>>();
@@ -263,141 +260,10 @@ pub struct PartialSuccess {
     pub issues: Vec<Issue>,
 }
 
-/// Some issue the user should address. Errors block composition, warnings do not.
-#[derive(Clone, Debug)]
-pub struct Issue {
-    pub code: String,
-    pub message: String,
-    pub locations: Vec<SubgraphLocation>,
-    pub severity: Severity,
-}
-
-/// A location in a subgraph's SDL
-#[derive(Clone, Debug)]
-pub struct SubgraphLocation {
-    /// This field is an Option to support the lack of subgraph names in
-    /// existing composition errors. New composition errors should always
-    /// include a subgraph name.
-    pub subgraph: Option<String>,
-    pub range: Option<Range<LineColumn>>,
-}
-
-#[derive(Clone, Copy, Debug, Eq, PartialEq)]
-pub enum Severity {
-    Error,
-    Warning,
-}
-
-impl From<ValidationSeverity> for Severity {
-    fn from(severity: ValidationSeverity) -> Self {
-        match severity {
-            ValidationSeverity::Error => Severity::Error,
-            ValidationSeverity::Warning => Severity::Warning,
-        }
-    }
-}
-
-impl From<Severity> for BuildMessageLevel {
-    fn from(severity: Severity) -> Self {
-        match severity {
-            Severity::Error => BuildMessageLevel::Error,
-            Severity::Warning => BuildMessageLevel::Warn,
-        }
-    }
-}
-
-impl From<Issue> for BuildMessage {
-    fn from(issue: Issue) -> Self {
-        BuildMessage {
-            level: issue.severity.into(),
-            message: issue.message,
-            code: Some(issue.code.to_string()),
-            locations: issue
-                .locations
-                .into_iter()
-                .map(|location| location.into())
-                .collect(),
-            schema_coordinate: None,
-            step: None,
-            other: Default::default(),
-        }
-    }
-}
-
-impl From<SubgraphLocation> for BuildMessageLocation {
-    fn from(location: SubgraphLocation) -> Self {
-        BuildMessageLocation {
-            subgraph: location.subgraph,
-            start: location.range.as_ref().map(|range| BuildMessagePoint {
-                line: Some(range.start.line),
-                column: Some(range.start.column),
-                start: None,
-                end: None,
-            }),
-            end: location.range.as_ref().map(|range| BuildMessagePoint {
-                line: Some(range.end.line),
-                column: Some(range.end.column),
-                start: None,
-                end: None,
-            }),
-            source: None,
-            other: Default::default(),
-        }
-    }
-}
-
-impl SubgraphLocation {
-    fn from_ast(node: SubgraphASTNode) -> Option<Self> {
-        Some(Self {
-            subgraph: node.subgraph,
-            range: node.loc.and_then(|node_loc| {
-                Some(Range {
-                    start: LineColumn {
-                        line: node_loc.start_token.line?,
-                        column: node_loc.start_token.column?,
-                    },
-                    end: LineColumn {
-                        line: node_loc.end_token.line?,
-                        column: node_loc.end_token.column?,
-                    },
-                })
-            }),
-        })
-    }
-}
-
-impl From<GraphQLError> for Issue {
-    fn from(error: GraphQLError) -> Issue {
-        Issue {
-            code: error
-                .extensions
-                .map(|extension| extension.code)
-                .unwrap_or_default(),
-            message: error.message,
-            severity: Severity::Error,
-            locations: error
-                .nodes
-                .unwrap_or_default()
-                .into_iter()
-                .filter_map(SubgraphLocation::from_ast)
-                .collect(),
-        }
-    }
-}
-
-impl From<CompositionHint> for Issue {
-    fn from(hint: CompositionHint) -> Issue {
-        Issue {
-            code: hint.definition.code,
-            message: hint.message,
-            severity: Severity::Warning,
-            locations: hint
-                .nodes
-                .unwrap_or_default()
-                .into_iter()
-                .filter_map(SubgraphLocation::from_ast)
-                .collect(),
-        }
+fn convert_severity(severity: ValidationSeverity) -> Severity {
+    match severity {
+        ValidationSeverity::Error => Severity::Error,
+        ValidationSeverity::Warning => Severity::Warning,
     }
 }
 
@@ -422,58 +288,3 @@ fn satisfiability_result_into_issues(
         Err(issue) => Either::Right(once(issue)),
     }
 }
-
-impl From<BuildError> for Issue {
-    fn from(error: BuildError) -> Issue {
-        Issue {
-            code: error
-                .code
-                .unwrap_or_else(|| "UNKNOWN_ERROR_CODE".to_string()),
-            message: error.message.unwrap_or_else(|| "Unknown error".to_string()),
-            locations: error
-                .nodes
-                .unwrap_or_default()
-                .into_iter()
-                .map(Into::into)
-                .collect(),
-            severity: Severity::Error,
-        }
-    }
-}
-
-impl From<BuildHint> for Issue {
-    fn from(hint: BuildHint) -> Issue {
-        Issue {
-            code: hint.code.unwrap_or_else(|| "UNKNOWN_HINT_CODE".to_string()),
-            message: hint.message,
-            locations: hint
-                .nodes
-                .unwrap_or_default()
-                .into_iter()
-                .map(Into::into)
-                .collect(),
-            severity: Severity::Warning,
-        }
-    }
-}
-
-impl From<BuildMessageLocation> for SubgraphLocation {
-    fn from(location: BuildMessageLocation) -> Self {
-        Self {
-            subgraph: location.subgraph,
-            range: location.start.and_then(|start| {
-                let end = location.end?;
-                Some(Range {
-                    start: LineColumn {
-                        line: start.line?,
-                        column: start.column?,
-                    },
-                    end: LineColumn {
-                        line: end.line?,
-                        column: end.column?,
-                    },
-                })
-            }),
-        }
-    }
-}

apollo-federation-types/CHANGELOG.md

@@ -2,6 +2,12 @@
 
 Not every version is listed here because versions before 0.14.0 did not have a changelog.
 
+## 0.15.1 - Unreleased
+
+### Features
+
+- Added new `composition` module behind the `composition` Cargo feature for types related to composition (previously in the `apollo-composition` crate).
+
 ## 0.15.0
 
 ### Breaking changes

apollo-federation-types/Cargo.toml

@@ -15,10 +15,14 @@ default = ["config", "build", "build_plugin"]
 
 build = ["serde_json"]
 build_plugin = ["serde_json"]
+composition = ["apollo-compiler"]
 config = ["log", "thiserror", "serde_yaml", "url", "serde_with"]
 json_schema = ["schemars"]
 
 [dependencies]
+# only used for composition
+apollo-compiler = { workspace = true, optional = true }
+
 # config and build dependencies
 serde = { version = "1", features = ["derive"] }
 schemars = { version = "0.8.21", optional = true, features = ["url"] }