Document the 'checks' module #3611

andreyaksenov · 2023-08-08T09:24:40Z

Added a new page for the checks module:
https://docs.d.tarantool.io/en/doc/embedded-checks/reference/reference_lua/checks/

Note that new documentation topics now references testable code snippets from the code_snippets folder.

doc/reference/reference_lua/checks.rst

DifferentialOrange

LGTM from my side.

The documentation seems really nice and useful. It would be good to add a documentation page link to the checks repo README after merge.

andreyaksenov · 2023-08-14T14:42:17Z

LGTM from my side.

The documentation seems really nice and useful. It would be good to add a documentation page link to the checks repo README after merge.

Thanks, will make a PR after merging.

p7nov

Generally OK, but some places seem hard to read; I suggested simpler/shorter wordings.

p7nov · 2023-08-15T06:38:17Z

doc/reference/reference_lua/checks.rst

+**Since:** :doc:`2.11.0 </release/2.11.0>`
+
+The ``checks`` module provides the ability to check the types of arguments passed to a Lua function.
+You need to call the :ref:`checks(type_1, ...) <checks-checks>` function inside the target Lua function and pass :ref:`one or more <checks_number_of_arguments>` type qualifiers to check the corresponding argument types.


Nit: I'd start with "to check a function's argument types, call ..." to be more goal-oriented. In the current wording, the path to reader's goal is too long.

p7nov · 2023-08-15T06:39:44Z

doc/reference/reference_lua/checks.rst

+-   A :ref:`string type qualifier <checks_string_type_qualifier>` checks whether a function's argument conforms to the specified type.
+-   A :ref:`table type qualifier <checks_table_type_qualifiers>` checks whether the values of a table passed as an argument conform to the specified types.


Maybe add short examples directly in the text?

p7nov · 2023-08-15T06:45:09Z

doc/reference/reference_lua/checks.rst

+Loading checks
+--------------
+
+The :ref:`checks API <checks_api_reference>` is available in a script without loading the module using ``require('checks')``.


Maybe say that require is needed only if the module is installed from repo?

I actually don't get how our devs decide which modules need require and which don't, but the explicit sentence "this module doesn't need require" looks a bit awkward.

I actually don't get how our devs decide which modules need require and which don't

Someone just decides that's rawset(_G, 'checks', checks) in the module is good idea five years ago and then no one breaks it since they don't have a reason to do so. Actually, in case of repo-installed checks you also don't need to require it more than once. Once you have a require in any of your scripts, it is available in any code called later. It's just that built-in modules are required inside the core Tarantool code, so the first require already had happened for any application developer.

Actually, I prefer calling local checks = require('checks') in any script, but, well, it is a fact that it is not actually needed. My original comment was related to checkers -- it is available in global after first require('checks') (which had happen already internally), but you cannot do require('checkers') -- it is only in global.

p7nov · 2023-08-15T07:00:20Z

doc/reference/reference_lua/checks.rst

+Number of arguments to check
+----------------------------
+
+Depending on the number of arguments to be checked, you can pass one or more type qualifiers to the :ref:`checks(type_1, ...) <checks-checks>` function.


The sentence reads a bit too formal.
Maybe smth simpler, like: "For each argument to check, you need to specify its own type qualifier"?

p7nov · 2023-08-15T07:05:08Z

doc/reference/reference_lua/checks.rst

+    :end-before: -- Tests
+    :dedent:
+
+To turn off checking of specific arguments, use the :ref:`? placeholder <checks_any_type>`.


Do I get it right that ? is for checking arbitrary subsets of arguments? For example, in f(a, b, c, d), check only a and d? If yes, then maybe show an example?

Also, skip checking seems better to me here than turn off checking.

Do I get it right that ? is for checking arbitrary subsets of arguments? For example, in f(a, b, c, d), check only a and d? If yes, then maybe show an example?

Input count must match with checks expected count (excluding varargs).

tarantool> g = function(a, b) checks('number', 'number') end --- ... tarantool> g(1, 2) --- ... tarantool> g(1) --- - error: '[string "g = function(a, b) checks(''number'', ''number'')..."]:1: bad argument #2 to nil (number expected, got nil)' ... tarantool> g = function(a, b) checks('number') end --- ... tarantool> g(1, 2) --- - error: '[string "g = function(a, b) checks(''number'') end"]:1: checks: argument "b" is not checked' ... tarantool> g = function(a, b) checks('number', '?') end --- ... tarantool> g(1) --- ...

So, in case you want to check some args and you do not care about other args, you must check them with '?' since you cannot simply skip them. It makes even more sence in case you don't want to check some argument in the middle. It is similar for table fields -- if you want to allow optional field X with arbitrary value, you use '?'.

Do I get it right that ? is for checking arbitrary subsets of arguments? For example, in f(a, b, c, d), check only a and d? If yes, then maybe show an example?

You are right. Will update the example to make it more clear.

p7nov · 2023-08-15T07:23:04Z

doc/reference/reference_lua/checks.rst

+            -   See also
+
+        *   -   ``checks('datetime')``
+            -   Check whether the specified value is :ref:`datetime_object <datetime_obj>`


I don't see much point in repeating the same sentence in all rows changing only the type (which is obvious from the first column). Can we restructure the table somehow?

Auto-generated lists of API members often look like this. I think, it's not a problem.

p7nov · 2023-08-15T07:24:56Z

doc/reference/reference_lua/checks.rst

+***************
+
+A string type qualifier can accept the name of a custom function that performs arbitrary validations.
+To achieve this, the function should be added to the :ref:`checkers <checks-checkers>` table and should return ``true`` if the value is valid.


Maybe rephrase to direct instruction: "write a function and add to the table"?

p7nov · 2023-08-15T07:25:45Z

doc/reference/reference_lua/checks.rst

+
+.. _checks_any_type:
+
+Turning off argument checking


Turn off > Skip?

Not sure am I right or not but to me skip has a slightly different meaning and suits better for cases when we have some kind of a loop. Here, we turn off checking of the specified argument.

I see it this way:

by default, no args are checked - checking is turned off for all of them.

turn off checking means return the default (no check) after turning on checking.

but this text tells about the case when nobody turned on checking for these args :)

"skip" came to me because I see args as a sequence. So I read a check call with a ? like this: check arg1, check arg2, skip arg3, check arg4.

Now I got this :) Will update

p7nov · 2023-08-15T07:27:10Z

doc/reference/reference_lua/checks.rst

+Table type qualifier
+--------------------
+
+The type qualifier may be a table. In this case, the following checks are made:


This description wasn't enough for me to understand what this is used for. Maybe write that it's for cases when arguments are tables?

Replaced with a sentence from intro.

p7nov · 2023-08-15T07:30:43Z

doc/reference/reference_lua/checks.rst

+
+.. NOTE::
+
+    A table type qualifier may be nested and use tables, too.


"Table qualifiers can be nested"?

andreyaksenov force-pushed the embedded-checks branch 17 times, most recently from 5a29a4c to 3b3a197 Compare August 9, 2023 08:03

andreyaksenov linked an issue Aug 9, 2023 that may be closed by this pull request

embedded checks #3331

Closed

andreyaksenov force-pushed the embedded-checks branch from 3b3a197 to ff8866d Compare August 9, 2023 09:54

andreyaksenov changed the title ~~Embedded checks~~ Document the 'checks' module Aug 9, 2023

andreyaksenov force-pushed the embedded-checks branch 10 times, most recently from 507d790 to e9f2a64 Compare August 10, 2023 12:12

andreyaksenov force-pushed the embedded-checks branch 13 times, most recently from ead4aa2 to 9f2005c Compare August 14, 2023 07:46

andreyaksenov requested a review from DifferentialOrange August 14, 2023 08:06

DifferentialOrange reviewed Aug 14, 2023

View reviewed changes

doc/reference/reference_lua/checks.rst Show resolved Hide resolved

doc/reference/reference_lua/checks.rst Show resolved Hide resolved

doc/reference/reference_lua/checks.rst Outdated Show resolved Hide resolved

doc/reference/reference_lua/checks.rst Show resolved Hide resolved

andreyaksenov force-pushed the embedded-checks branch 5 times, most recently from 3e881a1 to 0334428 Compare August 14, 2023 14:30

DifferentialOrange approved these changes Aug 14, 2023

View reviewed changes

andreyaksenov requested a review from p7nov August 14, 2023 14:50

p7nov approved these changes Aug 15, 2023

View reviewed changes

andreyaksenov force-pushed the embedded-checks branch 2 times, most recently from 069c74c to 1147c98 Compare August 15, 2023 08:52

Embedded checks

4f60474

andreyaksenov force-pushed the embedded-checks branch from 1147c98 to 4f60474 Compare August 15, 2023 09:12

andreyaksenov merged commit 0684ec7 into latest Aug 15, 2023

andreyaksenov deleted the embedded-checks branch August 15, 2023 09:12

		- A :ref:`string type qualifier <checks_string_type_qualifier>` checks whether a function's argument conforms to the specified type.
		- A :ref:`table type qualifier <checks_table_type_qualifiers>` checks whether the values of a table passed as an argument conform to the specified types.


		.. NOTE::

		A table type qualifier may be nested and use tables, too.

Document the 'checks' module #3611

Document the 'checks' module #3611

Uh oh!

Conversation

andreyaksenov commented Aug 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DifferentialOrange left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andreyaksenov commented Aug 14, 2023

Uh oh!

p7nov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DifferentialOrange Aug 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

p7nov Aug 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

andreyaksenov commented Aug 8, 2023 •

edited

Loading

DifferentialOrange left a comment •

edited

Loading

DifferentialOrange Aug 15, 2023 •

edited

Loading

p7nov Aug 15, 2023 •

edited

Loading