Skip to content

Commit

Permalink
Assorted API tweaks + prepare apollo_compiler@1.0.0 beta.19 (#885)
Browse files Browse the repository at this point in the history
Moved from the crate root to the new(ly public) `apollo_compiler::parser` module:
* `Parser`
* `SourceFile`
* `SourceMap`
* `FileId`

Moved to the `parser` module and renamed:
* `NodeLocation` → `SourceSpan`
* `GraphQLLocation` → `LineColumn`

Added a few helper methods:
* `executable::OperationMap::from_one`
* `schema::SchemaDefinition::iter_root_operations`
* `schema::ExtendedType::is_leaf`
  • Loading branch information
SimonSapin authored Jul 19, 2024
1 parent 527bcb2 commit 13f4aa1
Show file tree
Hide file tree
Showing 39 changed files with 518 additions and 450 deletions.
48 changes: 47 additions & 1 deletion crates/apollo-compiler/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
## Maintenance
## Documentation-->

# [x.x.x] (unreleased) - 2024-mm-dd
# [1.0.0-beta.19](https://crates.io/crates/apollo-compiler/1.0.0-beta.19) - 2024-07-19

## BREAKING

Expand All @@ -38,15 +38,61 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
This change makes `get_mut()` borrow only `doc.operations` instead of the entire document,
making it possible to also mutate `doc.fragments` during that mutable borrow.

- **Move or rename some import paths - [SimonSapin] in [pull/885].**
Moved from the crate root to the new(ly public) `apollo_compiler::parser` module:
* `Parser`
* `SourceFile`
* `SourceMap`
* `FileId`

Moved to the `parser` module and renamed:
* `NodeLocation``SourceSpan`
* `GraphQLLocation``LineColumn`

- **Return ranges of line/column locations - [dylan-apollo] in [pull/861].**
`SourceSpan` contains a file ID and a range of UTF-8 offsets within that file.
Various APIs can convert offsets to line and column numbers,
but often only considered the start offset.
They are now changed to consider the range of start and end positions.
Added, returning `Option<Range<LineColumn>>`:
* `SourceSpan::line_column_range`
* `Diagnostic::line_column_range`
* `Name::line_column_range`

Removed:
* `LineColumn::from_node`
* `Diagnostic::get_line_column`
* `SourceFile::get_line_column`

- **Use a fast hasher - [o0Ignition0o] in [pull/881].**
Configured all hash-based collections used in apollo-compiler to use `ahash`,
which is faster than the default standard library hasher.
`apollo_compiler::collections` provides type aliases for collections
configured with the same hasher as collections in various parts of the public API.

## Features

- **Add a few helper methods - [SimonSapin] in [pull/885]:**
* `executable::OperationMap::from_one`
* `schema::SchemaDefinition::iter_root_operations()`
* `schema::ExtendedType::is_leaf`

## Fixes

- **Fix potential hash collision bug in validation - [goto-bus-stop] in [pull/878]**
- **Fix validation for undefined variables nested in values - [goto-bus-stop] in [pull/885]**

[SimonSapin]: https://github.com/SimonSapin
[goto-bus-stop]: https://github.com/goto-bus-stop
[o0Ignition0o]: https://github.com/o0Ignition0o
[dylan-apollo]: https://github.com/dylan-apollo
[pull/861]: https://github.com/apollographql/apollo-rs/pull/861
[pull/877]: https://github.com/apollographql/apollo-rs/pull/877
[pull/878]: https://github.com/apollographql/apollo-rs/pull/878
[pull/879]: https://github.com/apollographql/apollo-rs/pull/879
[pull/881]: https://github.com/apollographql/apollo-rs/pull/881
[pull/883]: https://github.com/apollographql/apollo-rs/pull/883
[pull/885]: https://github.com/apollographql/apollo-rs/pull/885


# [1.0.0-beta.18](https://crates.io/crates/apollo-compiler/1.0.0-beta.18) - 2024-06-27
Expand Down
2 changes: 1 addition & 1 deletion crates/apollo-compiler/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "apollo-compiler"
version = "1.0.0-beta.18" # When bumping, also update README.md
version = "1.0.0-beta.19" # When bumping, also update README.md
authors = ["Irina Shestak <shestak.irina@gmail.com>"]
license = "MIT OR Apache-2.0"
repository = "https://github.com/apollographql/apollo-rs"
Expand Down
2 changes: 1 addition & 1 deletion crates/apollo-compiler/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ Or add this to your `Cargo.toml` for a manual installation:
# Just an example, change to the necessary package version.
# Using an exact dependency is recommended for beta versions
[dependencies]
apollo-compiler = "=1.0.0-beta.18"
apollo-compiler = "=1.0.0-beta.19"
```

## Rust versions
Expand Down
12 changes: 6 additions & 6 deletions crates/apollo-compiler/src/ast/from_cst.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use crate::ast;
use crate::ast::Document;
use crate::validation::FileId;
use crate::parser::FileId;
use crate::parser::SourceMap;
use crate::parser::SourceSpan;
use crate::Node;
use crate::NodeLocation;
use crate::SourceMap;
use apollo_parser::cst;
use apollo_parser::cst::CstNode;
use apollo_parser::SyntaxNode;
Expand All @@ -28,7 +28,7 @@ pub(crate) trait Convert {
}

fn with_location<T>(file_id: FileId, syntax_node: &SyntaxNode, node: T) -> Node<T> {
Node::new_parsed(node, NodeLocation::new(file_id, syntax_node))
Node::new_parsed(node, SourceSpan::new(file_id, syntax_node))
}

/// Convert and collect, silently skipping entries with conversion errors
Expand Down Expand Up @@ -429,7 +429,7 @@ impl Convert for cst::Description {
fn convert(&self, file_id: FileId) -> Option<Self::Target> {
Some(Node::new_str_parsed(
&String::from(self.string_value()?),
NodeLocation::new(file_id, self.syntax()),
SourceSpan::new(file_id, self.syntax()),
))
}
}
Expand Down Expand Up @@ -762,7 +762,7 @@ impl Convert for cst::Name {
type Target = crate::Name;

fn convert(&self, file_id: FileId) -> Option<Self::Target> {
let loc = NodeLocation::new(file_id, self.syntax());
let loc = SourceSpan::new(file_id, self.syntax());
let token = &self.syntax().first_token()?;
let str = token.text();
debug_assert!(crate::Name::is_valid_syntax(str));
Expand Down
8 changes: 4 additions & 4 deletions crates/apollo-compiler/src/ast/impls.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use super::*;
use crate::name;
use crate::node::NodeLocation;
use crate::parser::Parser;
use crate::parser::SourceSpan;
use crate::schema::SchemaBuilder;
use crate::validation::DiagnosticList;
use crate::validation::Valid;
use crate::validation::WithErrors;
use crate::ExecutableDocument;
use crate::Parser;
use crate::Schema;
use std::fmt;
use std::hash;
Expand Down Expand Up @@ -212,7 +212,7 @@ impl Definition {
}
}

pub fn location(&self) -> Option<NodeLocation> {
pub fn location(&self) -> Option<SourceSpan> {
match self {
Self::OperationDefinition(def) => def.location(),
Self::FragmentDefinition(def) => def.location(),
Expand Down Expand Up @@ -853,7 +853,7 @@ impl EnumValueDefinition {
}

impl Selection {
pub fn location(&self) -> Option<NodeLocation> {
pub fn location(&self) -> Option<SourceSpan> {
match self {
Self::Field(field) => field.location(),
Self::FragmentSpread(fragment) => fragment.location(),
Expand Down
6 changes: 4 additions & 2 deletions crates/apollo-compiler/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
//!
//! ## Parsing
//!
//! Start with [`Document::parse`], or [`Parser`][crate::Parser] to change the parser configuration.
//! Start with [`Document::parse`], or [`Parser`][crate::parser::Parser]
//! to change the parser configuration.
//!
//! ## Structural sharing and mutation
//!
Expand All @@ -31,6 +32,7 @@
//! that has chaining methods for setting serialization configuration,
//! and also implements `Display` and `ToString`.

use crate::parser::SourceMap;
use crate::Name;
use crate::Node;

Expand All @@ -46,7 +48,7 @@ pub struct Document {
/// this map contains one entry for that file and its ID.
///
/// The document may have been modified since.
pub sources: crate::SourceMap,
pub sources: SourceMap,

pub definitions: Vec<Definition>,
}
Expand Down
36 changes: 18 additions & 18 deletions crates/apollo-compiler/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
//! To use pretty-printing in custom errors, implement the [`ToCliReport`] trait.
//!
//! ```rust
//! use apollo_compiler::NodeLocation;
//! use apollo_compiler::parser::SourceSpan;
//! use apollo_compiler::Schema;
//! use apollo_compiler::Name;
//! use apollo_compiler::diagnostic::CliReport;
Expand All @@ -18,13 +18,13 @@
//! InvalidCase { name: Name },
//! #[error("Missing @specifiedBy directive on scalar {name}")]
//! NoSpecifiedBy {
//! location: Option<NodeLocation>,
//! location: Option<SourceSpan>,
//! name: Name,
//! },
//! }
//!
//! impl ToCliReport for LintError {
//! fn location(&self) -> Option<NodeLocation> {
//! fn location(&self) -> Option<SourceSpan> {
//! match self {
//! LintError::InvalidCase { name } => name.location(),
//! LintError::NoSpecifiedBy { location, .. } => *location,
Expand Down Expand Up @@ -52,12 +52,12 @@
//! ready for formatting:
//!
//! ```rust
//! # use apollo_compiler::{NodeLocation, Schema, diagnostic::{ToCliReport, CliReport}};
//! # use apollo_compiler::{parser::SourceSpan, Schema, diagnostic::{ToCliReport, CliReport}};
//! # #[derive(Debug, thiserror::Error)]
//! # #[error("")]
//! # struct LintError {}
//! # impl ToCliReport for LintError {
//! # fn location(&self) -> Option<NodeLocation> { None }
//! # fn location(&self) -> Option<SourceSpan> { None }
//! # fn report(&self, _report: &mut CliReport) {}
//! # }
//! fn print_errors(schema: &Schema, errors: &[LintError]) {
Expand All @@ -68,15 +68,15 @@
//! }
//! ```
use crate::execution::GraphQLError;
use crate::execution::GraphQLLocation;
use crate::validation::FileId;
use crate::parser::FileId;
use crate::parser::LineColumn;
use crate::parser::SourceFile;
use crate::parser::SourceMap;
use crate::parser::SourceSpan;
#[cfg(doc)]
use crate::ExecutableDocument;
use crate::NodeLocation;
#[cfg(doc)]
use crate::Schema;
use crate::SourceFile;
use crate::SourceMap;
use ariadne::ColorGenerator;
use ariadne::ReportKind;
use std::cell::Cell;
Expand Down Expand Up @@ -128,7 +128,7 @@ pub enum Color {
pub trait ToCliReport: fmt::Display {
/// Return the main location for this error. May be `None` if a location doesn't make sense for
/// the particular error.
fn location(&self) -> Option<NodeLocation>;
fn location(&self) -> Option<SourceSpan>;

/// Fill in the report with source code labels.
///
Expand Down Expand Up @@ -157,7 +157,7 @@ pub trait ToCliReport: fmt::Display {
}

impl<T: ToCliReport> ToCliReport for &T {
fn location(&self) -> Option<NodeLocation> {
fn location(&self) -> Option<SourceSpan> {
ToCliReport::location(*self)
}

Expand All @@ -166,12 +166,12 @@ impl<T: ToCliReport> ToCliReport for &T {
}
}

/// An ariadne span type. We avoid implementing `ariadne::Span` for `NodeLocation`
/// An ariadne span type. We avoid implementing `ariadne::Span` for `SourceSpan`
/// so ariadne doesn't leak into the public API of apollo-compiler.
type AriadneSpan = (FileId, Range<usize>);

/// Translate a NodeLocation into an ariadne span type.
fn to_span(location: NodeLocation) -> Option<AriadneSpan> {
/// Translate a SourceSpan into an ariadne span type.
fn to_span(location: SourceSpan) -> Option<AriadneSpan> {
let start = location.offset();
let end = location.end_offset();
Some((location.file_id, start..end))
Expand Down Expand Up @@ -201,7 +201,7 @@ impl<'s> CliReport<'s> {
/// Source files can be obtained from [`Schema::sources`] or [`ExecutableDocument::sources`].
pub fn builder(
sources: &'s SourceMap,
main_location: Option<NodeLocation>,
main_location: Option<SourceSpan>,
color: Color,
) -> Self {
let (file_id, range) = main_location
Expand Down Expand Up @@ -241,7 +241,7 @@ impl<'s> CliReport<'s> {
}

/// Add a label at a given location. If the location is `None`, the message is discarded.
pub fn with_label_opt(&mut self, location: Option<NodeLocation>, message: impl ToString) {
pub fn with_label_opt(&mut self, location: Option<SourceSpan>, message: impl ToString) {
if let Some(span) = location.and_then(to_span) {
self.report.add_label(
ariadne::Label::new(span)
Expand Down Expand Up @@ -326,7 +326,7 @@ impl<T: ToCliReport> std::error::Error for Diagnostic<'_, T> {}

impl<T: ToCliReport> Diagnostic<'_, T> {
/// Get the line and column numbers where this diagnostic spans.
pub fn line_column_range(&self) -> Option<Range<GraphQLLocation>> {
pub fn line_column_range(&self) -> Option<Range<LineColumn>> {
self.error.location()?.line_column_range(self.sources)
}

Expand Down
Loading

0 comments on commit 13f4aa1

Please sign in to comment.