Skip to content

Commit

Permalink
Fix some clippy lints
Browse files Browse the repository at this point in the history
Reviewed By: tyao1

Differential Revision: D64553850

fbshipit-source-id: 9e0097b25d588e10218df1724c0ba8ef2d530c81
  • Loading branch information
gordyf authored and facebook-github-bot committed Oct 17, 2024
1 parent 4776175 commit 495d06f
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 48 deletions.
14 changes: 7 additions & 7 deletions compiler/crates/graphql-ir/src/associated_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use std::hash::Hash;

pub trait AssociatedData: Send + Sync + fmt::Debug + AsAny {
fn clone_box(&self) -> Box<dyn AssociatedData>;
fn eq_box(&self, other: &Box<dyn AssociatedData>) -> bool;
fn eq_box(&self, other: &dyn AssociatedData) -> bool;
fn hash_box(&self) -> u64;
}
impl dyn AssociatedData {
Expand All @@ -27,7 +27,7 @@ impl Clone for Box<dyn AssociatedData> {
}
}

impl PartialEq for Box<dyn AssociatedData> {
impl PartialEq for dyn AssociatedData {
fn eq(&self, other: &Self) -> bool {
self.eq_box(other)
}
Expand Down Expand Up @@ -58,7 +58,7 @@ macro_rules! associated_data_impl {
Box::new(self.clone())
}

fn eq_box(&self, other: &Box<dyn $crate::AssociatedData>) -> bool {
fn eq_box(&self, other: &dyn $crate::AssociatedData) -> bool {
other
.downcast_ref::<Self>()
.map_or(false, |other| other == self)
Expand All @@ -77,12 +77,12 @@ macro_rules! associated_data_impl {
}
}

impl Into<$crate::Directive> for $name {
fn into(self) -> $crate::Directive {
impl From<$name> for $crate::Directive {
fn from(item: $name) -> Self {
$crate::Directive {
name: $crate::reexport::WithLocation::generated(Self::directive_name()),
name: $crate::reexport::WithLocation::generated($name::directive_name()),
arguments: Vec::new(),
data: Some(Box::new(self)),
data: Some(Box::new(item)),
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@ struct AssignableFragmentSpread<'s> {

impl<'s> AssignableFragmentSpread<'s> {
/// 1. Validate that the assignable fragment does not have @skip/@defer, and
/// is not within an inline fragment with directives, and is nested in a linked field
/// is not within an inline fragment with directives, and is nested in a linked field
/// 2. Mark the enclosing linked field as containing an assignable fragment spread.
/// This later results in an __id (clientid_field) selection being added to the linked
/// field.
/// This later results in an __id (clientid_field) selection being added to the linked
/// field.
fn validate_nesting_and_mark_enclosing_linked_field(
&mut self,
fragment_spread: &FragmentSpread,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ impl Transformer for GenerateLiveQueryMetadata {

if polling_interval.is_none() && config_id.is_none() {
self.errors.push(Diagnostic::error(
ValidationMessage::LiveQueryTransformMissingConfig {
LiveQueryTransformValidationMessage::MissingConfig {
query_name: operation.name.item,
},
live_query_directive.name.location,
Expand All @@ -87,7 +87,7 @@ impl Transformer for GenerateLiveQueryMetadata {
Value::Constant(ConstantValue::Int(value)) => value,
_ => {
self.errors.push(Diagnostic::error(
ValidationMessage::LiveQueryTransformInvalidPollingInterval {
LiveQueryTransformValidationMessage::InvalidPollingInterval {
query_name: operation.name.item,
},
polling_interval.value.location,
Expand All @@ -109,7 +109,7 @@ impl Transformer for GenerateLiveQueryMetadata {
Some(value) => value,
None => {
self.errors.push(Diagnostic::error(
ValidationMessage::LiveQueryTransformInvalidConfigId {
LiveQueryTransformValidationMessage::InvalidConfigId {
query_name: operation.name.item,
},
config_id.value.location,
Expand Down Expand Up @@ -143,19 +143,19 @@ impl Transformer for GenerateLiveQueryMetadata {

#[derive(Error, Debug, serde::Serialize)]
#[serde(tag = "type")]
enum ValidationMessage {
enum LiveQueryTransformValidationMessage {
#[error(
"Live query expects 'polling_interval' or 'config_id' as an argument to @live_query to for root field {query_name}"
)]
LiveQueryTransformMissingConfig { query_name: OperationDefinitionName },
MissingConfig { query_name: OperationDefinitionName },

#[error(
"Expected the 'polling_interval' argument to @live_query to be a literal number for root field {query_name}"
)]
LiveQueryTransformInvalidPollingInterval { query_name: OperationDefinitionName },
InvalidPollingInterval { query_name: OperationDefinitionName },

#[error(
"Expected the 'config_id' argument to @live_query to be a literal string for root field {query_name}"
)]
LiveQueryTransformInvalidConfigId { query_name: OperationDefinitionName },
InvalidConfigId { query_name: OperationDefinitionName },
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ use crate::util::format_provided_variable_name;
/// \[provided_variable_name\] --> __pv__\[module_name\]
/// - Remove provided variables from (local) argument definitions
/// - Add provided variables to list of used global variables
///
/// apply_fragment_arguments depends on provide_variable_fragment_transform
pub fn provided_variable_fragment_transform(program: &Program) -> DiagnosticsResult<Program> {
let mut transform = ProvidedVariableFragmentTransform::new(&program.schema);
Expand Down
18 changes: 9 additions & 9 deletions compiler/crates/relay-transforms/src/required_directive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ use lazy_static::lazy_static;
use requireable_field::RequireableField;
use requireable_field::RequiredMetadata;

use self::validation_message::ValidationMessage;
use self::validation_message::RequiredDirectiveValidationMessage;
use crate::DirectiveFinder;
use crate::FragmentAliasMetadata;

Expand Down Expand Up @@ -128,7 +128,7 @@ impl<'program> RequiredDirective<'program> {
fn assert_not_within_abstract_inline_fragment(&mut self, directive_location: Location) {
if self.within_abstract_inline_fragment {
self.errors.push(Diagnostic::error(
ValidationMessage::RequiredWithinAbstractInlineFragment,
RequiredDirectiveValidationMessage::WithinAbstractInlineFragment,
// TODO(T70172661): Also reference the location of the inline fragment, once they have a location.
directive_location,
))
Expand All @@ -139,7 +139,7 @@ impl<'program> RequiredDirective<'program> {
if let Some(location) = self.parent_inline_fragment_directive {
self.errors.push(
Diagnostic::error(
ValidationMessage::RequiredWithinInlineDirective,
RequiredDirectiveValidationMessage::WithinInlineDirective,
directive_location,
)
.annotate("The fragment is annotated as @inline here.", location),
Expand All @@ -154,7 +154,7 @@ impl<'program> RequiredDirective<'program> {
if previous_metadata.action != current_metadata.action {
self.errors.push(
Diagnostic::error(
ValidationMessage::RequiredActionMismatch {
RequiredDirectiveValidationMessage::ActionMismatch {
field_name: current.field_name.item,
},
previous_metadata.action_location,
Expand All @@ -168,7 +168,7 @@ impl<'program> RequiredDirective<'program> {
} else {
self.errors.push(
Diagnostic::error(
ValidationMessage::RequiredFieldMismatch {
RequiredDirectiveValidationMessage::FieldMismatch {
field_name: current.field_name.item,
},
previous.field_name.location,
Expand All @@ -179,7 +179,7 @@ impl<'program> RequiredDirective<'program> {
} else if current.required.is_some() {
self.errors.push(
Diagnostic::error(
ValidationMessage::RequiredFieldMismatch {
RequiredDirectiveValidationMessage::FieldMismatch {
field_name: current.field_name.item,
},
current.field_name.location,
Expand Down Expand Up @@ -238,7 +238,7 @@ impl<'program> RequiredDirective<'program> {
if required_child.required.action < parent_action {
self.errors.push(
Diagnostic::error(
ValidationMessage::RequiredFieldInvalidNesting {
RequiredDirectiveValidationMessage::FieldInvalidNesting {
suggested_action: required_child.required.action.into(),
},
required_metadata.action_location,
Expand Down Expand Up @@ -270,7 +270,7 @@ impl<'program> RequiredDirective<'program> {
if let Some(other_parent) = self.path_required_map.get(&field_path) {
self.errors.push(
Diagnostic::error(
ValidationMessage::RequiredFieldMissing {
RequiredDirectiveValidationMessage::FieldMissing {
field_name: required_child.field_name.item,
},
required_child.field_name.location,
Expand All @@ -295,7 +295,7 @@ impl<'program> RequiredDirective<'program> {
if !self.current_node_required_children.contains_key(path) {
self.errors.push(
Diagnostic::error(
ValidationMessage::RequiredFieldMissing {
RequiredDirectiveValidationMessage::FieldMissing {
field_name: required_child.field_name.item,
},
required_child.field_name.location,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use graphql_ir::ScalarField;
use intern::string_key::StringKey;
use schema::SDLSchema;

use super::validation_message::ValidationMessage;
use super::validation_message::RequiredDirectiveValidationMessage;
use super::ACTION_ARGUMENT;
use super::REQUIRED_DIRECTIVE_NAME;
use crate::RequiredAction;
Expand Down Expand Up @@ -69,7 +69,7 @@ fn get_action_argument(
let maybe_action_arg = required_directive.arguments.named(*ACTION_ARGUMENT);
let action_arg = maybe_action_arg.ok_or_else(|| {
Diagnostic::error(
ValidationMessage::RequiredActionArgumentRequired,
RequiredDirectiveValidationMessage::ActionArgumentRequired,
required_directive.name.location,
)
})?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,35 +10,35 @@ use thiserror::Error;

#[derive(Error, Debug, serde::Serialize)]
#[serde(tag = "type")]
pub(super) enum ValidationMessage {
pub(super) enum RequiredDirectiveValidationMessage {
#[error(
"Unexpected @required within inline fragment on an abstract type. At runtime we cannot know if this field is null, or if it's missing because the inline fragment did not match. Consider using `@alias` to give your inline fragment a name."
)]
RequiredWithinAbstractInlineFragment,
WithinAbstractInlineFragment,

#[error("@required is not supported within @inline fragments.")]
RequiredWithinInlineDirective,
WithinInlineDirective,

#[error("Missing `action` argument. @required expects an `action` argument")]
RequiredActionArgumentRequired,
ActionArgumentRequired,

#[error(
"All references to a @required field must have matching `action` arguments. The `action` used for '{field_name}'"
)]
RequiredActionMismatch { field_name: StringKey },
ActionMismatch { field_name: StringKey },

#[error(
"All references to a field must have matching @required declarations. The field '{field_name}` is @required here"
)]
RequiredFieldMismatch { field_name: StringKey },
FieldMismatch { field_name: StringKey },

#[error(
"@required fields must be included in all instances of their parent. The field '{field_name}` is marked as @required here"
)]
RequiredFieldMissing { field_name: StringKey },
FieldMissing { field_name: StringKey },

#[error(
"A @required field may not have an `action` less severe than that of its @required parent. This @required directive should probably have `action: {suggested_action}`"
)]
RequiredFieldInvalidNesting { suggested_action: StringKey },
FieldInvalidNesting { suggested_action: StringKey },
}
18 changes: 6 additions & 12 deletions compiler/crates/relay-transforms/src/skip_redundant_nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ use crate::RelayLocationAgnosticBehavior;
*
*
* 2. Inline fragments and conditions introduce the possibility for duplication
* at different levels of the tree. Whenever a selection is fetched in a parent,
* it is redundant to also fetch it in a child:
* at different levels of the tree. Whenever a selection is fetched in a parent,
* it is redundant to also fetch it in a child:
*
*
* fragment Foo on FooType {
Expand Down Expand Up @@ -479,21 +479,15 @@ impl SkipRedundantNodesTransform {
*/
fn get_partitioned_selections<'a>(&self, selections: &'a [Selection]) -> Vec<&'a Selection> {
let mut result = Vec::with_capacity(selections.len());
unsafe { result.set_len(selections.len()) };
let mut non_field_index = selections
.iter()
.filter(|sel| self.is_selection_linked_or_scalar(sel))
.count();
let mut field_index = 0;
let mut non_field_selections = Vec::new();
for sel in selections.iter() {
if self.is_selection_linked_or_scalar(sel) {
result[field_index] = sel;
field_index += 1;
result.push(sel);
} else {
result[non_field_index] = sel;
non_field_index += 1;
non_field_selections.push(sel);
}
}
result.extend(non_field_selections);
result
}

Expand Down

0 comments on commit 495d06f

Please sign in to comment.