Remove remains of hook_field_schema()

Patch no longer applies cleanly. Re-rolled against 8.2.x.

Patch applies cleanly and looks good.

+++ b/core/modules/field/src/Entity/FieldStorageConfig.php
@@ -167,12 +167,12 @@ class FieldStorageConfig extends ConfigEntityBase implements FieldStorageConfigI
+   * field type in schema() method to determine the actual set of indexes

Shouldn't this link to the fully qualified interface name + method?

Updated patch tested on D9.

Thanks for the quick response @andypost. Here is the updated patch.

Regarding #20

````
+++ b/core/modules/field/field.api.php
@@ -18,7 +18,7 @@
* In the Field API, each field has a type, which determines what kind of data
* (integer, string, date, etc.) the field can hold, which settings it provides,
* and so on. The data type(s) accepted by a field are defined in
- * hook_field_schema().
+ * hook_schema().
````

I am of the opinion that this place should mention "schema() method", propertyDefinitions() method appears a method for adding/specifying additional attributes for the field item.
In this block the schema method is been referenced with respect to data type(s) which is defined in schema().

Referenced:
core/lib/Drupal/Core/Field/Plugin/Field/FieldType/BooleanItem.php
core/modules/file/src/Plugin/Field/FieldType/FileItem.php
core/lib/Drupal/Core/Field/Plugin/Field/FieldType/UriItem.php

Thanks andypost for moving this to RTBC.

I think the wording must change:

- hook_schema()
+ schema() method

Kindly suggest and accordingly we may mark this for needs work?

Thanks.

update patch at #19

b/core/modules/field/field.api.php

replace: hook_field_schema()

with: schema() method

Thanks.

Hi @atul4drupal as per your suggestion i have updated the patch.

Hi Saurabh_sgh thanks for working on this. The changes look good.

It will however be more helpful if you ensure adding interdiff file.

apologies for the mess, I mistakenly have this set to needs work

Hi @atul4drupal I have added the interdiff.

patch #25 looks good to me also includes suggestion #24. Moving back to RTBC.

Thanks for fixing this; good find.

+++ b/core/lib/Drupal/Core/Field/FieldConfigBase.php
@@ -125,8 +125,8 @@
+   * defined by the "field schema" for the field type, as exposed by schema()

+++ b/core/modules/field/field.api.php
@@ -18,7 +18,7 @@
- * hook_field_schema().
+ * schema() method.

+++ b/core/modules/field/src/Entity/FieldStorageConfig.php
@@ -175,12 +175,12 @@ class FieldStorageConfig extends ConfigEntityBase implements FieldStorageConfigI
-   * field type in hook_field_schema() to determine the actual set of indexes
+   * field type in schema() method to determine the actual set of indexes
...
-   * defined by the field type in hook_field_schema(), are allowed.
+   * defined by the field type in schema() method, are allowed.

I thinnk we need to specify the interface with the fully qualified namespace for the method in the docs here. Should be just another set of small changes. Thanks!

Hi @xjm,

I hope this is the final change. I have updated the patch as per your suggestion.

Thanks

Movie again to RTBC as suggestions from #32 added in #33.

Thanks @Saurabh_sgh. Two more small things:

1. +++ b/core/lib/Drupal/Core/Field/FieldConfigBase.php
   @@ -125,8 +125,9 @@
   +   * \Drupal\Core\Field\FieldItemInterface::schema() method of the field. ¶
   
   +++ b/core/modules/field/src/Entity/FieldStorageConfig.php
   @@ -175,12 +175,13 @@ class FieldStorageConfig extends ConfigEntityBase implements FieldStorageConfigI
   +   * field type in \Drupal\Core\Field\FieldItemInterface::schema() method to ¶
   

   There are trailing spaces on these lines that need to be removed.

2. +++ b/core/modules/field/src/Entity/FieldStorageConfig.php
   @@ -175,12 +175,13 @@ class FieldStorageConfig extends ConfigEntityBase implements FieldStorageConfigI
   +   * defined by the field type in \Drupal\Core\Field\FieldItemInterface::schema()
   +   * method, are allowed.
   

   The first line here is now over 80 characters. It needs to be rewrapped.

Tagging novice since those are small cleanups. Thank you for working on this!

Removed trailing spaces and rewrapped the line.

The 2 points of @xjm are addressed.
Back to RTBC.

Regarding #7, #32 & #33:

Thanks for working on this & taking this forward, this looks very close.

Still not sure of the statement with respect to "FieldItemInterface".

The statement in :

b/core/modules/field/src/Entity/FieldStorageConfig.php

This set of indexes is merged with the "default" indexes specified by the field type in \Drupal\Core\Field\FieldItemInterface::schema() method to determine the actual set of indexes that get created.

Here this statement gives an impression as if the method is implemented in "FieldItemInterface" interface itself, wherein it only holds signature of the method.

Likewise at other places:
b/core/modules/field/field.api.php
b/core/lib/Drupal/Core/Field/FieldConfigBase.php

These statements with respect to "FieldItemInterface::schema()", I believe still need some fine improvement so that it implies the interface implementing 'snippetItem' classes schema method, which actually holds the field schema definition and not the interface.

@atul4drupal,

Here this statement gives an impression as if the method is implemented in "FieldItemInterface" interface itself, wherein it only holds signature of the method.

Interface's comment should describe the abstract purpose of this method/interface. In \Drupal\Core\Field\FieldItemInterface::schema() it describe like

* Returns the schema for the field.

And the concrete comment will talk about the implementation specifics of the method/class in the context of the interface's purpose. which is clearly mentioned in comment "This set of indexes is merged with the "default" indexes specified by the field type in \Drupal\Core\Field\FieldItemInterface::schema() method"

Regarding #41,

#41.1 - This I believe is not about what the comment for schema() method in FieldItemInterface reads.

#41.2 - I am only concerned with the sense that current used statement derives. It definitely need fine improvement so that it implies the 'class implementing the interface method', currently it gives sense as if all implementations of schema methods will return same data.

Also if we have to call the schema() method we will call it from the class implementing the interface and not the one mentioned in the interface(if by any means we may do that), this is why I believe the statement needs fine tuning to get the right sense out of it.

for example something in the lines of : indexes specified by field type in the <'class' OR 'snippetItem class'> implementing \Drupal\Core\Field\FieldItemInterface::schema() method

updated patch for review.
Taking cue from https://www.drupal.org/docs/creating-custom-modules/creating-custom-field-types-widgets-and-formatters/overview-creating-a for using "SnippetsItem" keyword to represent the field-type class.

Reagrding #43 Its my bad .. seems I didn't have the page refreshed before posting the patch.

Adding interdiff for #38 & #44

Hi @atul4drupal,

* and so on. The data type(s) accepted by a field are defined in
 * SnippetsItem class implementing

There is no such class SnippetsItem present in the core. So, I don't think it is useful to mention.

Regarding #47: Thanks for the review and feedback, seems I haven't clarified myself at #44 comment. SnippetsItem is a generic keyword used to represent "field type" class, and this has been termed so in referenced page at #44, and in current context it makes sense to use generic word to represent the classes implementing schema() method from the \Drupal\Core\Field\FieldItemInterface interface.

For this reason you will not find a class named "SnippetsItem" nor in core neither elsewhere. Suggestion are welcome to improve the statement to derive correct meaning out of it.

And yes we can think if only using "class" will suffice or shall we make it more obvious by using "SnippetsItem" keyword, this I will leave it to the community at large to decide and accordingly can take call based on feedback from others.

Though personally I am of the opinion to using SnippetsItem keyword (even I had this thought of what will be more appropriate here)

Yes you are correct its a generic word, for me it will make more sense to just write

 * and so on. The data type(s) accepted by a field are defined in a
 * class implementing \Drupal\Core\Field\FieldItemInterface::schema() method.

instead of

 * and so on. The data type(s) accepted by a field are defined in
 * SnippetsItem class implementing
 * \Drupal\Core\Field\FieldItemInterface::schema() method.

which uses a SnippetsItem as in #44.

Similarly at other occurrences.

This key word is never used anywhere to generalize the class. Therefore we should keep it same as we have done earlier.

#50 Thanks for clarifying your stand.
I am open to have it done either way actually as mentioned in #42, but would suggest waiting for more feedback on this and progress accordingly.

Submitting a patch covering points from #50.

#52 thanks for the patch, I appreciate your effort.

Just a few updates:

1)

b/core/lib/Drupal/Core/Field/FieldConfigBase.php
+ * defined by the "field schema" for the field type, as exposed in a class
+ * class implementing \Drupal\Core\Field\FieldItemInterface::schema() method.

here we have class word mentioned twice... also its better if we use "as exposed in the class" instead of in a class.

2) b/core/modules/field/field.api.php

+ * and so on. The data type(s) accepted by a field are defined in a class

here also change "in a class" with "in the class"

3) core/modules/field/src/Entity/FieldStorageConfig.php

+ * field type in a class implementing
change "in a class" with "in the class"

+ *defined by the field type in a class implementing
change "in a class" with "in the class"

Once done I believe we should be good.

Thanks for your valuable feedback, I have worked on all the points mentioned and here the patch.

#54 looks clean.

Committed and pushed 8d67348e9a to 9.1.x and 0285b30825 to 9.0.x and 9522bf29e0 to 8.9.x. Thanks!

Backported to 8.9.x since hook_field_schema() was removed from Drupal 8 :)

diff --git a/core/modules/field/src/Entity/FieldStorageConfig.php b/core/modules/field/src/Entity/FieldStorageConfig.php
index 5dc4b3c204..250edd6e8d 100644
--- a/core/modules/field/src/Entity/FieldStorageConfig.php
+++ b/core/modules/field/src/Entity/FieldStorageConfig.php
@@ -176,7 +176,7 @@ class FieldStorageConfig extends ConfigEntityBase implements FieldStorageConfigI
    *
    * This set of indexes is merged with the "default" indexes specified by the
    * field type in the class implementing
-   * \Drupal\Core\Field\FieldItemInterface::schema() method, to determine the
+   * \Drupal\Core\Field\FieldItemInterface::schema() method to determine the
    * actual set of indexes that get created.
    *
    * The indexes are defined using the same definition format as Schema API

I don't think the comma here is correct so I removed it on commit.