Merge branch 'ld-enforce-graphql-description-starts' into 'master'

Enforce GraphQL descriptions styleguide for A/The

See merge request gitlab-org/gitlab!67473

doc/development/api_graphql_styleguide.md

@@ -837,7 +837,7 @@ descriptions:
 - Mention the name of the resource in the description. Example:
   `'Labels of the issue'` (issue being the resource).
 - Use `"{x} of the {y}"` where possible. Example: `'Title of the issue'`.
-  Do not start descriptions with `The`.
+  Do not start descriptions with `The` or `A`, for consistency and conciseness.
 - Descriptions of `GraphQL::Types::Boolean` fields should answer the question: "What does
   this field do?". Example: `'Indicates project has a Git repository'`.
 - Always include the word `"timestamp"` when describing an argument or

rubocop/cop/graphql/descriptions.rb

@@ -32,7 +32,7 @@
 #
 #     field :some_field,
 #       GraphQL::Types::String,
-#       description: "A thorough and compelling description."
+#       description: "Thorough and compelling description."
 #   end
 #
 #   class GoodEnum
@@ -43,8 +43,10 @@ module RuboCop
   module Cop
     module Graphql
       class Descriptions < RuboCop::Cop::Cop
-        MSG_NO_DESCRIPTION = 'Please add a `description` property.'
-        MSG_NO_PERIOD = '`description` strings must end with a `.`.'
+        MSG_STYLE_GUIDE_LINK = 'See the description style guide: https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#description-style-guide'
+        MSG_NO_DESCRIPTION = "Please add a `description` property. #{MSG_STYLE_GUIDE_LINK}"
+        MSG_NO_PERIOD = "`description` strings must end with a `.`. #{MSG_STYLE_GUIDE_LINK}"
+        MSG_BAD_START = "`description` strings should not start with \"A...\" or \"The...\". #{MSG_STYLE_GUIDE_LINK}"
 
         def_node_matcher :graphql_describable?, <<~PATTERN
           (send nil? {:field :argument :value} ...)
@@ -75,6 +77,7 @@ module RuboCop
           return add_offense(node, location: :expression, message: MSG_NO_DESCRIPTION) unless description
 
           add_offense(node, location: :expression, message: MSG_NO_PERIOD) if no_period?(description)
+          add_offense(node, location: :expression, message: MSG_BAD_START) if bad_start?(description)
         end
 
         # Autocorrect missing periods at end of description.
@@ -100,12 +103,19 @@ module RuboCop
         end
 
         def no_period?(description)
-          # Test that the description node is a `:str` (as opposed to
-          # a `#copy_field_description` call) before checking.
-          description.type == :str && !description.value.strip.end_with?('.')
+          string?(description) && !description.value.strip.end_with?('.')
         end
 
-        # Returns a Parser::Source::Range that ends just before the final String delimiter.
+        def bad_start?(description)
+          string?(description) && description.value.strip.downcase.start_with?('a ', 'the ')
+        end
+
+        # Returns true if `description` node is a `:str` (as opposed to a `#copy_field_description` call)
+        def string?(description)
+          description.type == :str
+        end
+
+        # Returns a `Parser::Source::Range` that ends just before the final `String` delimiter.
         def before_end_quote(string)
           return string.source_range.adjust(end_pos: -1) unless string.heredoc?
 

spec/rubocop/cop/graphql/descriptions_spec.rb

@@ -12,7 +12,7 @@ RSpec.describe RuboCop::Cop::Graphql::Descriptions do
         module Types
           class FakeType < BaseObject
             field :a_thing,
-            ^^^^^^^^^^^^^^^ Please add a `description` property.
+            ^^^^^^^^^^^^^^^ #{described_class::MSG_NO_DESCRIPTION}
               GraphQL::Types::String,
               null: false
           end
@@ -25,10 +25,38 @@ RSpec.describe RuboCop::Cop::Graphql::Descriptions do
         module Types
           class FakeType < BaseObject
             field :a_thing,
-            ^^^^^^^^^^^^^^^ `description` strings must end with a `.`.
+            ^^^^^^^^^^^^^^^ #{described_class::MSG_NO_PERIOD}
               GraphQL::Types::String,
               null: false,
-              description: 'A descriptive description'
+              description: 'Description of a thing'
+          end
+        end
+      TYPE
+    end
+
+    it 'adds an offense when description begins with "A"' do
+      expect_offense(<<~TYPE)
+        module Types
+          class FakeType < BaseObject
+            field :a_thing,
+            ^^^^^^^^^^^^^^^ #{described_class::MSG_BAD_START}
+              GraphQL::Types::String,
+              null: false,
+              description: 'A description of the thing.'
+          end
+        end
+      TYPE
+    end
+
+    it 'adds an offense when description begins with "The"' do
+      expect_offense(<<~TYPE)
+        module Types
+          class FakeType < BaseObject
+            field :a_thing,
+            ^^^^^^^^^^^^^^^ #{described_class::MSG_BAD_START}
+              GraphQL::Types::String,
+              null: false,
+              description: 'The description of the thing.'
           end
         end
       TYPE
@@ -41,7 +69,7 @@ RSpec.describe RuboCop::Cop::Graphql::Descriptions do
             field :a_thing,
               GraphQL::Types::String,
               null: false,
-              description: 'A descriptive description.'
+              description: 'Description of a thing.'
           end
         end
       TYPE
@@ -64,7 +92,7 @@ RSpec.describe RuboCop::Cop::Graphql::Descriptions do
         module Types
           class FakeType < BaseObject
             argument :a_thing,
-            ^^^^^^^^^^^^^^^^^^ Please add a `description` property.
+            ^^^^^^^^^^^^^^^^^^ #{described_class::MSG_NO_DESCRIPTION}
               GraphQL::Types::String,
               null: false
           end
@@ -77,7 +105,7 @@ RSpec.describe RuboCop::Cop::Graphql::Descriptions do
         module Types
           class FakeType < BaseObject
             argument :a_thing,
-            ^^^^^^^^^^^^^^^^^^ `description` strings must end with a `.`.
+            ^^^^^^^^^^^^^^^^^^ #{described_class::MSG_NO_PERIOD}
               GraphQL::Types::String,
               null: false,
               description: 'Behold! A description'
@@ -86,6 +114,34 @@ RSpec.describe RuboCop::Cop::Graphql::Descriptions do
       TYPE
     end
 
+    it 'adds an offense when description begins with "A"' do
+      expect_offense(<<~TYPE)
+        module Types
+          class FakeType < BaseObject
+            argument :a_thing,
+            ^^^^^^^^^^^^^^^^^^ #{described_class::MSG_BAD_START}
+              GraphQL::Types::String,
+              null: false,
+              description: 'A description.'
+          end
+        end
+      TYPE
+    end
+
+    it 'adds an offense when description begins with "The"' do
+      expect_offense(<<~TYPE)
+        module Types
+          class FakeType < BaseObject
+            argument :a_thing,
+            ^^^^^^^^^^^^^^^^^^ #{described_class::MSG_BAD_START}
+              GraphQL::Types::String,
+              null: false,
+              description: 'The description.'
+          end
+        end
+      TYPE
+    end
+
     it 'does not add an offense when description is correct' do
       expect_no_offenses(<<~TYPE.strip)
         module Types
@@ -106,7 +162,7 @@ RSpec.describe RuboCop::Cop::Graphql::Descriptions do
         module Types
           class FakeEnum < BaseEnum
             value 'FOO', value: 'foo'
-            ^^^^^^^^^^^^^^^^^^^^^^^^^ Please add a `description` property.
+            ^^^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG_NO_DESCRIPTION}
           end
         end
       TYPE
@@ -117,7 +173,29 @@ RSpec.describe RuboCop::Cop::Graphql::Descriptions do
         module Types
           class FakeEnum < BaseEnum
             value 'FOO', value: 'foo', description: 'bar'
-            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `description` strings must end with a `.`.
+            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG_NO_PERIOD}
+          end
+        end
+      TYPE
+    end
+
+    it 'adds an offense when description begins with "The"' do
+      expect_offense(<<~TYPE.strip)
+        module Types
+          class FakeEnum < BaseEnum
+            value 'FOO', value: 'foo', description: 'The description.'
+            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG_BAD_START}
+          end
+        end
+      TYPE
+    end
+
+    it 'adds an offense when description begins with "A"' do
+      expect_offense(<<~TYPE.strip)
+        module Types
+          class FakeEnum < BaseEnum
+            value 'FOO', value: 'foo', description: 'A description.'
+            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG_BAD_START}
           end
         end
       TYPE
@@ -150,7 +228,7 @@ RSpec.describe RuboCop::Cop::Graphql::Descriptions do
         module Types
           class FakeType < BaseObject
             field :a_thing,
-            ^^^^^^^^^^^^^^^ `description` strings must end with a `.`.
+            ^^^^^^^^^^^^^^^ #{described_class::MSG_NO_PERIOD}
               GraphQL::Types::String,
               null: false,
               description: 'Behold! A description'
@@ -175,7 +253,7 @@ RSpec.describe RuboCop::Cop::Graphql::Descriptions do
         module Types
           class FakeType < BaseObject
             field :a_thing,
-            ^^^^^^^^^^^^^^^ `description` strings must end with a `.`.
+            ^^^^^^^^^^^^^^^ #{described_class::MSG_NO_PERIOD}
               GraphQL::Types::String,
               null: false,
               description: <<~DESC