diff --git a/docs/changelog.rst b/docs/changelog.rst index 2c09418..295783a 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -8,7 +8,7 @@ Unreleased +++++++++++++++++++++++++++++++++++ * New rule set :doc:`rules/dag` to validate DAG templates and their tasks. -* Redesign :doc:`rules/workflow-variable` to improve validation of Argo workflow variable references: +* Redesign :doc:`rules/variable` to improve validation of Argo workflow variable references: * Rule ``VAR001`` has been renamed to :rule:`var101`. * Rule ``VAR002`` has been renamed to :rule:`var201`. diff --git a/docs/index.rst b/docs/index.rst index 05de4e2..2777dfe 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -160,9 +160,9 @@ For more information on how to use Tugboat, runs its help command: rules/manifest-errors rules/step rules/template + rules/variable rules/workflow rules/workflow-template - rules/workflow-variable .. toctree:: :hidden: diff --git a/docs/rules/dag.rst b/docs/rules/dag.rst index ea45597..2211406 100644 --- a/docs/rules/dag.rst +++ b/docs/rules/dag.rst @@ -143,7 +143,7 @@ Code ``DAG`` is used for errors related to the `DAG`_ in a `template`_. .. rule:: DAG301 Invalid parameter reference A task input parameter contains a reference that cannot be resolved. - This rule builds on :rule:`VAR002` but narrows the report to the task argument where the typo or mismatch occurs. + This rule builds on :rule:`VAR201` but narrows the report to the task argument where the typo or mismatch occurs. .. rule:: DAG302 Invalid artifact reference @@ -357,7 +357,7 @@ Code ``DAG`` is used for errors related to the `DAG`_ in a `template`_. .. code-block:: yaml :caption: ❌ Example manifest that violates this rule - :emphasize-lines: 18 + :emphasize-lines: 12 apiVersion: argoproj.io/v1alpha1 kind: Workflow diff --git a/docs/rules/step.rst b/docs/rules/step.rst index 7926f1f..9df98ce 100644 --- a/docs/rules/step.rst +++ b/docs/rules/step.rst @@ -149,14 +149,14 @@ Code ``STP`` is used for errors related to the `steps`_ in a `template`_. Found invalid parameter reference in the step input parameter. - This rule is a variation of :rule:`VAR002`. + This rule is a variation of :rule:`VAR201`. It is triggered when a step input parameter references an invalid objective. .. rule:: STP302 Invalid artifact reference Found invalid artifact reference in the step input artifact. - This rule is a variation of :rule:`VAR002`. + This rule is a variation of :rule:`VAR201`. It is triggered when a step input artifact references an invalid objective. .. code-block:: yaml diff --git a/docs/rules/template.rst b/docs/rules/template.rst index 9cc6d0f..5fc21ce 100644 --- a/docs/rules/template.rst +++ b/docs/rules/template.rst @@ -177,7 +177,7 @@ Rules Found invalid parameter reference in the template input parameter. - This rule is a variation of :rule:`VAR002`. + This rule is a variation of :rule:`VAR201`. It is triggered when a template input parameter references an invalid objective: .. code-block:: yaml diff --git a/docs/rules/variable.rst b/docs/rules/variable.rst index 587a975..3f00751 100644 --- a/docs/rules/variable.rst +++ b/docs/rules/variable.rst @@ -123,7 +123,7 @@ The code ``VAR`` identifies potential issues with `workflow variables`_, includi When a tag contains only a simple variable name (e.g., ``{{ foo }}``), it is unlikely to be an Argo variable. .. code-block:: yaml - :emphasize-lines: 14 + :emphasize-lines: 11 apiVersion: argoproj.io/v1alpha1 kind: Workflow diff --git a/docs/rules/workflow-variable.rst b/docs/rules/workflow-variable.rst deleted file mode 100644 index 161647e..0000000 --- a/docs/rules/workflow-variable.rst +++ /dev/null @@ -1,74 +0,0 @@ -Workflow Variable Rules (``VAR``) -================================= - -The code ``VAR`` identifies potential issues with `workflow variables`_, including syntax errors, misused functions, and other related problems. - -.. _workflow variables: https://argo-workflows.readthedocs.io/en/latest/variables/ - -Known limitations ------------------ - -Currently, Tugboat supports only `simple template tags`_, and does not support `expression template tags`_. - -This means: - -.. code-block:: none - - {{ inputs.parameters.foo }} can be checked - - but {{= inputs.parameters.foo }} will be ignored - -We are planning to add support for expression template tags in an upcoming release. - -.. _simple template tags: https://argo-workflows.readthedocs.io/en/latest/variables/#simple -.. _expression template tags: https://argo-workflows.readthedocs.io/en/latest/variables/#expression - - -Rules ------ - -.. rule:: VAR001 Syntax error - - This error occurs when a workflow variable fails to parse due to a syntax error. - - Note that most parsers report syntax errors as soon as they encounter the first issue. - This means the error message might not always indicate the exact location of the problem, but it provides a useful starting point for debugging. - -.. rule:: VAR002 Misused reference - - A reference used in the manifest is not defined in the current context. - - Tugboat checks the references used in the manifest against a list of references from the `official documentation `_. - If a reference used in the manifest is not found in the defined references, an error is reported. - - .. code-block:: yaml - :emphasize-lines: 17 - - apiVersion: argoproj.io/v1alpha1 - kind: Workflow - metadata: - generateName: test- - spec: - entrypoint: whalesay - templates: - - name: whalesay - inputs: - artifacts: - - name: message - raw: - data: Hello Tugboat! - container: - image: docker/whalesay:latest - command: [cowsay] - args: ["{{ inputs.artifacts.message }}"] - - In the example above, ``inputs.artifacts.message`` is invalid because referencing artifacts in this field is not allowed. - - .. admonition:: Limited validation of step outputs - :class: note - - Tugboat can only validate references within the same manifest. - - The outputs of a step are defined by the template it refers to. - Tugboat cannot validate these outputs because their definitions are not included in the same manifest. - This means Tugboat cannot verify references that point to the outputs of other steps. diff --git a/docs/violations.rst b/docs/violations.rst index db4f4f4..750cb59 100644 --- a/docs/violations.rst +++ b/docs/violations.rst @@ -27,7 +27,7 @@ The comment may include the rule codes (separated by commas) for the violations The comment should be placed on the same line as the section where the violation occurs, or the parent level of the section. -The following example demonstrates how to ignore :rule:`var002` in the Argo workflow manifest. +The following example demonstrates how to ignore :rule:`var202` in the Argo workflow manifest. We want to ignore this violation because the template file is intended to be a Jinja template, rather than an Argo tag template. .. code-block:: yaml @@ -47,7 +47,7 @@ We want to ignore this violation because the template file is intended to be a J - name: template path: /tmp/template.j2 raw: - data: | # noqa: TPL202; this is a Jinja template + data: | # noqa: VAR202; this is a Jinja template Hello {{name}} - name: data path: /tmp/data.json diff --git a/tests/analyzers/test_container.py b/tests/analyzers/test_container.py index 46e0427..1d34ce6 100644 --- a/tests/analyzers/test_container.py +++ b/tests/analyzers/test_container.py @@ -3,15 +3,37 @@ def test_analyze_template(diagnoses_logger): - diagnoses = analyze_yaml_stream(MANIFEST_INVALID_REFERENCE) + diagnoses = analyze_yaml_stream( + """ + apiVersion: argoproj.io/v1alpha1 + kind: Workflow + metadata: + name: test + spec: + entrypoint: main + templates: + - name: main + script: + image: busybox + source: | + echo "{{ inputs.parameters.foo }}" + resources: + requests: + memory: 100Gi + cpu: "1.5" + limits: + memory: 10Gi + cpu: 1000m + """ + ) diagnoses_logger(diagnoses) assert ( IsPartialModel( { - "code": "VAR002", + "code": "VAR201", "loc": ("spec", "templates", 0, "script", "source"), - "input": "{{ inputs.parameters.foo }}", + "input": "inputs.parameters.foo", } ) in diagnoses @@ -54,29 +76,6 @@ def test_analyze_template(diagnoses_logger): ) -MANIFEST_INVALID_REFERENCE = """ -apiVersion: argoproj.io/v1alpha1 -kind: Workflow -metadata: - name: test -spec: - entrypoint: main - templates: - - name: main - script: - image: busybox - source: | - echo "{{ inputs.parameters.foo }}" - resources: - requests: - memory: 100Gi - cpu: "1.5" - limits: - memory: 10Gi - cpu: 1000m -""" - - def test_check_input_artifacts(diagnoses_logger): diagnoses = analyze_yaml_stream(MANIFEST_MISSING_INPUT_PATH) diagnoses_logger(diagnoses) diff --git a/tests/analyzers/test_metrics.py b/tests/analyzers/test_metrics.py index f1cbc13..f34a27e 100644 --- a/tests/analyzers/test_metrics.py +++ b/tests/analyzers/test_metrics.py @@ -1,6 +1,6 @@ from dirty_equals import IsPartialDict -from tests.dirty_equals import ContainsSubStrings +from tests.dirty_equals import HasSubstring from tugboat.analyzers.metrics import check_prometheus from tugboat.references import Context from tugboat.schemas.metrics import Prometheus @@ -46,7 +46,7 @@ def test_invalid_metric_name_1(self): { "code": "internal:invalid-metric-name", "loc": ("name",), - "msg": ContainsSubStrings("Metric name 'test/metric' is invalid."), + "msg": HasSubstring("Metric name 'test/metric' is invalid."), "input": "test/metric", } ), @@ -70,7 +70,7 @@ def test_invalid_metric_name_2(self): { "code": "internal:invalid-metric-name", "loc": ("name",), - "msg": ContainsSubStrings("Metric name is too long."), + "msg": HasSubstring("Metric name is too long."), "input": NAME, } ), @@ -98,7 +98,7 @@ def test_invalid_label_names(self): { "code": "internal:invalid-metric-label-name", "loc": ("labels", 0, "key"), - "msg": ContainsSubStrings( + "msg": HasSubstring( "Label name 'test/label' in metric 'test_metric' is invalid." ), "input": "test/label", @@ -108,7 +108,7 @@ def test_invalid_label_names(self): { "code": "internal:invalid-metric-label-name", "loc": ("labels", 1, "key"), - "msg": ContainsSubStrings( + "msg": HasSubstring( "Label name '__reserved' in metric 'test_metric' is invalid." ), "input": "__reserved", @@ -118,7 +118,7 @@ def test_invalid_label_names(self): { "code": "internal:invalid-metric-label-value", "loc": ("labels", 2, "value"), - "msg": ContainsSubStrings( + "msg": HasSubstring( "Label value for label 'valid_label' in metric 'test_metric' is empty." ), }, diff --git a/tests/analyzers/test_step.py b/tests/analyzers/test_step.py index 568364b..6d4a978 100644 --- a/tests/analyzers/test_step.py +++ b/tests/analyzers/test_step.py @@ -342,13 +342,28 @@ def test_check_referenced_template(caplog: pytest.LogCaptureFixture, diagnoses_l def test_check_fields_references(diagnoses_logger): - diagnoses = analyze_yaml_stream(MANIFEST_FIELDS_REFERENCES) + diagnoses = analyze_yaml_stream( + """ + apiVersion: argoproj.io/v1alpha1 + kind: Workflow + metadata: + generateName: exit-handler-step-level- + spec: + entrypoint: main + templates: + - name: main + steps: + - - name: hello + when: "{{ count }} > 0" + template: print-message + """ + ) diagnoses_logger(diagnoses) assert ( IsPartialModel( { - "code": "VAR002", + "code": "VAR202", "loc": ("spec", "templates", 0, "steps", 0, 0, "when"), } ) @@ -356,22 +371,6 @@ def test_check_fields_references(diagnoses_logger): ) -MANIFEST_FIELDS_REFERENCES = """ -apiVersion: argoproj.io/v1alpha1 -kind: Workflow -metadata: - generateName: exit-handler-step-level- -spec: - entrypoint: main - templates: - - name: main - steps: - - - name: hello - when: "{{ count }} > 0" - template: print-message -""" - - def test_check_inline_template(diagnoses_logger): diagnoses = analyze_yaml_stream(MANIFEST_INLINE_TEMPLATE) diagnoses_logger(diagnoses) diff --git a/tests/analyzers/test_template.py b/tests/analyzers/test_template.py index d6a6732..f5d85b6 100644 --- a/tests/analyzers/test_template.py +++ b/tests/analyzers/test_template.py @@ -32,10 +32,10 @@ def test_check_field_references(self, diagnoses_logger): assert ( IsPartialModel( { - "code": "VAR002", + "code": "VAR201", "loc": ("spec", "templates", 0, "container", "args", 1), "msg": "The parameter reference 'inputs.parameters.command' used in template 'container-template' is invalid.", - "fix": "{{ inputs.parameters.cmd }}", + "fix": "inputs.parameters.cmd", } ) in diagnoses @@ -43,10 +43,10 @@ def test_check_field_references(self, diagnoses_logger): assert ( IsPartialModel( { - "code": "VAR002", + "code": "VAR201", "loc": ("spec", "templates", 1, "script", "source"), "msg": "The parameter reference 'inputs.artifacts.data' used in template 'script-template' is invalid.", - "fix": "{{ inputs.artifacts.data.path }}", + "fix": "inputs.artifacts.data.path", } ) in diagnoses @@ -152,7 +152,7 @@ def test_check_duplicate_task_names(self, diagnoses_logger): command: [ python ] args: - -c - - '{{ inputs.parameters.command }}' # VAR002 + - '{{ inputs.parameters.command }}' # VAR201 - name: script-template inputs: artifacts: @@ -161,7 +161,7 @@ def test_check_duplicate_task_names(self, diagnoses_logger): image: python:alpine3.13 command: [ python ] source: |- - print('Hello world, {{ inputs.artifacts.data }}!') # VAR002 + print('Hello world, {{ inputs.artifacts.data }}!') # VAR201 """ MANIFEST_DUPLICATE_STEP_NAMES = """ @@ -208,7 +208,7 @@ def test_check_input_parameters_1(diagnoses_logger): valueFrom: path: /malformed # M102 - name: message # TPL102 - value: "{{ workflow.name " # VAR001 + value: "{{ workflow.name " # VAR101 - name: message-3 value: "{{ workflow.invalid}}" # - name: message-4 @@ -221,13 +221,12 @@ def test_check_input_parameters_1(diagnoses_logger): assert IsPartialModel(code="TPL102", loc=(*loc, 0, "name")) in diagnoses assert IsPartialModel(code="TPL102", loc=(*loc, 1, "name")) in diagnoses assert IsPartialModel(code="M102", loc=(*loc, 0, "valueFrom", "path")) in diagnoses - assert IsPartialModel(code="VAR001", loc=(*loc, 1, "value")) in diagnoses + assert IsPartialModel(code="VAR101", loc=(*loc, 1, "value")) in diagnoses assert ( IsPartialModel( code="TPL201", loc=(*loc, 2, "value"), msg="The parameter reference 'workflow.invalid' used in parameter 'message-3' is invalid.", - input="{{ workflow.invalid}}", ) in diagnoses ) @@ -307,7 +306,6 @@ def test_check_input_artifacts(diagnoses_logger): msg=HasSubstring( "The parameter reference 'workflow.namee' used in artifact 'data' is invalid." ), - fix="{{ workflow.name }}", ) in diagnoses ) @@ -344,7 +342,7 @@ def test_check_output_parameters(diagnoses_logger): - name: message # TPL104 - name: message # TPL104 valueFrom: - parameter: "{{ workflow.invalid}}" # VAR002 + parameter: "{{ workflow.invalid}}" # VAR201 - name: main suspend: {} @@ -364,7 +362,7 @@ def test_check_output_parameters(diagnoses_logger): assert IsPartialModel(code="M101", loc=(*loc, 0, "valueFrom")) in diagnoses assert ( - IsPartialModel(code="VAR002", loc=(*loc, 1, "valueFrom", "parameter")) + IsPartialModel(code="VAR201", loc=(*loc, 1, "valueFrom", "parameter")) in diagnoses ) @@ -401,7 +399,7 @@ def test_check_output_artifacts(diagnoses_logger): assert IsPartialModel(code="M101", loc=(*loc, 0, "archive")) in diagnoses assert IsPartialModel(code="M102", loc=(*loc, 0, "path")) in diagnoses - assert IsPartialModel(code="VAR002", loc=(*loc, 1, "from")) in diagnoses + assert IsPartialModel(code="VAR202", loc=(*loc, 1, "from")) in diagnoses def test_check_metrics(diagnoses_logger): @@ -431,56 +429,20 @@ def test_check_metrics(diagnoses_logger): - key: invalid-label value: "" counter: - value: "{{ outputs.parameters.no-param }}" # VAR002 + value: "{{ outputs.parameters.no-param }}" # VAR201 """ ) diagnoses_logger(diagnoses) + loc_prom = ("spec", "templates", 0, "metrics", "prometheus", 0) + assert IsPartialModel(code="TPL301", loc=(*loc_prom, "name")) in diagnoses assert ( - IsPartialModel( - { - "code": "TPL301", - "loc": ("spec", "templates", 0, "metrics", "prometheus", 0, "name"), - } - ) - in diagnoses + IsPartialModel(code="TPL302", loc=(*loc_prom, "labels", 0, "key")) in diagnoses ) - - loc_labels = ("spec", "templates", 0, "metrics", "prometheus", 0, "labels") assert ( - IsPartialModel( - { - "code": "TPL302", - "loc": (*loc_labels, 0, "key"), - } - ) + IsPartialModel(code="TPL303", loc=(*loc_prom, "labels", 0, "value")) in diagnoses ) assert ( - IsPartialModel( - { - "code": "TPL303", - "loc": (*loc_labels, 0, "value"), - } - ) - in diagnoses - ) - - assert ( - IsPartialModel( - { - "code": "VAR002", - "loc": ( - "spec", - "templates", - 0, - "metrics", - "prometheus", - 0, - "counter", - "value", - ), - } - ) - in diagnoses + IsPartialModel(code="VAR201", loc=(*loc_prom, "counter", "value")) in diagnoses ) diff --git a/tests/analyzers/test_template_tag.py b/tests/analyzers/test_template_tag.py index f598ae9..84d7d67 100644 --- a/tests/analyzers/test_template_tag.py +++ b/tests/analyzers/test_template_tag.py @@ -3,17 +3,97 @@ import lark import pytest from dirty_equals import IsPartialDict, IsStr +from pydantic import BaseModel from tests.dirty_equals import HasSubstring from tugboat.analyzers.template_tag import ( _check_simple_tag_reference, check_template_tags, + check_template_tags_recursive, parse_argo_template_tags, split_expr_membership, ) from tugboat.references import ReferenceCollection +class TestCheckTemplateTagsRecursive: + + def test(self): + class Class(BaseModel): + class Profile(BaseModel): + name: str + age: int # int => should be ignored + + name: str + teacher: Profile + students: list[Profile] + + model = Class.model_validate( + { + "name": "{{ inputs.parameters.class_name }}", + "teacher": { + "name": "Mr. {{ inputs.parameters.teacher_name }}", + "age": 40, + }, + "students": [ + {"name": "{{ inputs.parameters.student1_name }}", "age": 20}, + {"name": "{{ inputs.parameters.student2_name }}", "age": 20}, + ], + } + ) + + references = ReferenceCollection( + [ + ("inputs", "parameters", "teacher_name"), + ("inputs", "parameters", "student1_name"), + ] + ) + + diagnoses = list(check_template_tags_recursive(model, references)) + assert diagnoses == [ + IsPartialDict( + code="VAR201", + loc=("name",), + input="inputs.parameters.class_name", + fix="inputs.parameters.teacher_name", + ), + IsPartialDict( + code="VAR201", + loc=("students", 1, "name"), + input="inputs.parameters.student2_name", + fix="inputs.parameters.student1_name", + ), + ] + + def test_include(self): + class Model(BaseModel): + x: str + y: str + + model = Model.model_validate( + { + "x": "{{ inputs.parameters.name }}", + "y": "foobar", + } + ) + + diagnoses = list( + check_template_tags_recursive(model, ReferenceCollection(), include=["y"]) + ) + assert diagnoses == [] + + def test_exclude(self): + class Model(BaseModel): + x: str + + model = Model.model_validate({"x": "{{ inputs.parameters.name }}"}) + + diagnoses = list( + check_template_tags_recursive(model, ReferenceCollection(), exclude=["x"]) + ) + assert diagnoses == [] + + class TestParseArgoTemplateTags: def test_1(self): diff --git a/tests/console/test_mcp.py b/tests/console/test_mcp.py index 3d7ee0b..49f61f0 100644 --- a/tests/console/test_mcp.py +++ b/tests/console/test_mcp.py @@ -79,11 +79,11 @@ async def test_plain(self, tmp_path: Path): ), IsPartialDict( { - "code": "VAR002", + "code": "VAR201", "sourceNearby": ' args:\n - "{{ inputs.parameter.message }}"', "loc": ["spec", "templates", 0, "container", "args", 0], - "input": "{{ inputs.parameter.message }}", - "fix": "{{ inputs.parameters.message }}", + "input": "inputs.parameter.message", + "fix": "inputs.parameters.message", } ), ], diff --git a/tests/parsers/ast/test_argo_template.py b/tests/parsers/ast/test_argo_template.py deleted file mode 100644 index f416253..0000000 --- a/tests/parsers/ast/test_argo_template.py +++ /dev/null @@ -1,177 +0,0 @@ -import textwrap - -from dirty_equals import IsInstance - -import tugboat.parsers.lexer -from tugboat._vendor.pygments.token import Error, Name, Punctuation, Text, Whitespace -from tugboat.parsers.ast.argo_template import ( - Document, - ExpressionTag, - PlainText, - SimpleReferenceTag, -) -from tugboat.parsers.ast.core import Unexpected - - -class TestDocument: - - def parse(self, text: str) -> Document: - text = textwrap.dedent(text).strip() - - lexer = tugboat.parsers.lexer.ArgoTemplateLexer() - lexemes = list(lexer.get_tokens_unprocessed(text)) - - doc = Document.parse(lexemes) - assert isinstance(doc, Document) - assert lexemes == [] - - return doc - - def test_plain_text(self): - doc = self.parse("hello world") - assert doc.children == [PlainText(text="hello world")] - assert list(doc.iter_references()) == [] - - def test_reference_tag(self): - doc = self.parse("{{ foo }}") - assert doc.children == [SimpleReferenceTag(raw="{{ foo }}", reference=("foo",))] - assert list(doc.iter_references()) == [ - (IsInstance(SimpleReferenceTag), ("foo",)) - ] - - def test_expression_tag(self): - doc = self.parse("{{= foo(bar, baz) }}") - assert doc.children == [ExpressionTag(literal="{{= foo(bar, baz) }}")] - - def test_mixed(self): - doc = self.parse("Hello, {{ foo }}! I'm {{= bar }}.") - assert doc.children == [ - PlainText(text="Hello, "), - SimpleReferenceTag(raw="{{ foo }}", reference=("foo",)), - PlainText(text="! I'm "), - ExpressionTag(literal="{{= bar }}"), - PlainText(text="."), - ] - assert list(doc.iter_references()) == [ - (IsInstance(SimpleReferenceTag), ("foo",)) - ] - - def test_error_1(self): - doc = self.parse("Hello {{ foo !") - assert doc.children == [ - PlainText(text="Hello "), - Unexpected(pos=6, text="{{ foo "), - Unexpected(pos=13, text="!"), - ] - - def test_error_2(self): - doc = self.parse("Hello {{ foo.}} !") - assert doc.children == [ - PlainText(text="Hello "), - Unexpected(pos=6, text="{{ foo"), - Unexpected(pos=12, text="."), - Unexpected(pos=13, text="}}"), - PlainText(text=" !"), - ] - - -class TestPlainText: - - def test_matched_1(self): - lexemes = [ - (-1, Text, "Hello"), - (-1, Text, " "), - (-1, Text, "world"), - ] - - node = PlainText.parse(lexemes) - assert lexemes == [] - - assert isinstance(node, PlainText) - assert node.text == "Hello world" - - def test_matched_2(self): - lexemes = [ - (-1, Text, "Hello"), - (-1, Error, "#"), - ] - - node = PlainText.parse(lexemes) - assert len(lexemes) == 1 - - assert isinstance(node, PlainText) - assert node.text == "Hello" - - -class TestSimpleReferenceTag: - - def test_matched_1(self): - lexemes = [ - (-1, Punctuation, "{{"), - (-1, Whitespace, " "), - (-1, Name.Variable, "foo"), - (-1, Punctuation, "}}"), - ] - - node = SimpleReferenceTag.parse(lexemes) - assert lexemes == [] - - assert isinstance(node, SimpleReferenceTag) - assert str(node) == "{{ foo}}" - assert node.reference == ("foo",) - - def test_matched_2(self): - lexemes = [ - (-1, Punctuation, "{{"), - (-1, Name.Variable, "foo"), - (-1, Punctuation, "."), - (-1, Name.Variable, "bar"), - (-1, Whitespace, " "), - (-1, Punctuation, "}}"), - ] - - node = SimpleReferenceTag.parse(lexemes) - assert lexemes == [] - - assert isinstance(node, SimpleReferenceTag) - assert str(node) == "{{foo.bar }}" - assert node.reference == ("foo", "bar") - - def test_warning(self): - lexemes = [ - (-1, Punctuation, "{{="), - ] - assert SimpleReferenceTag.parse(lexemes) is None - - def test_error_general(self): - lexemes = [ - (-1, Punctuation, "{{"), - (-1, Name.Variable, "foo"), - (-1, Punctuation, "."), - (-1, Punctuation, "}}"), - ] - - node = SimpleReferenceTag.parse(lexemes) - - assert isinstance(node, Unexpected) - assert node.text == "{{foo" - - def test_error_missing_closing_tag(self): - lexemes = [ - (-1, Punctuation, "{{"), - (-1, Name.Variable, "foo"), - (-1, Punctuation, "."), - (-1, Name.Variable, "bar"), - ] - - node = SimpleReferenceTag.parse(lexemes) - assert lexemes == [] - - assert isinstance(node, Unexpected) - assert node.text == "{{foo.bar" - assert node.msg == "expect closing tag '}}'" - - def test_format(self): - assert SimpleReferenceTag.format(("foo", "bar")) == "{{ foo.bar }}" - assert SimpleReferenceTag.format(("foo",)) == "{{ foo }}" - assert SimpleReferenceTag.format(()) is None diff --git a/tests/parsers/ast/test_ast.py b/tests/parsers/ast/test_ast.py deleted file mode 100644 index 707dcfc..0000000 --- a/tests/parsers/ast/test_ast.py +++ /dev/null @@ -1,32 +0,0 @@ -from tugboat._vendor.pygments.token import Other -from tugboat.parsers.ast.core import Unexpected - - -class TestUnexpected: - def test_parse_1(self): - lexemes = [(0, Other, "hello"), (5, Other, "world")] - - node = Unexpected.parse(lexemes) - assert len(lexemes) == 1 - - assert isinstance(node, Unexpected) - assert node.pos == 0 - assert node.text == "hello" - - def test_parse_2(self): - node = Unexpected.parse([]) - assert node is None - - def test_create_1(self): - lexemes = [(0, Other, "hello"), (5, Other, "world")] - node = Unexpected.create(lexemes) - assert isinstance(node, Unexpected) - assert node.pos == 0 - assert node.text == "helloworld" - assert node.msg is None - - def test_create_2(self): - lexemes = [(0, Other, "hello"), (5, Other, "world")] - node = Unexpected.create(lexemes, "test error") - assert isinstance(node, Unexpected) - assert node.msg == "test error" diff --git a/tests/parsers/test_lexer.py b/tests/parsers/test_lexer.py deleted file mode 100644 index ae61335..0000000 --- a/tests/parsers/test_lexer.py +++ /dev/null @@ -1,58 +0,0 @@ -import textwrap - -from tugboat._vendor.pygments.token import Name, Other, Punctuation, Text, Whitespace -from tugboat.parsers.lexer import ArgoTemplateLexer - - -class TestArgoTemplateLexer: - - def lex(self, source: str) -> list[tuple[str, str]]: - lexer = ArgoTemplateLexer() - tokens = [] - for _, token, text in lexer.get_tokens_unprocessed(source): - tokens.append((token, text)) - return tokens - - def test_simple_reference(self): - tokens = self.lex("{{ inputs.parameters.message}}") - assert tokens == [ - (Punctuation, "{{"), - (Whitespace, " "), - (Name.Variable, "inputs"), - (Punctuation, "."), - (Name.Variable, "parameters"), - (Punctuation, "."), - (Name.Variable, "message"), - (Punctuation, "}}"), - ] - - def test_simple_reference_2(self): - tokens = self.lex( - textwrap.dedent( - """ - Hello, - This is a test for {{inputs.parameters.message }}! - """ - ).strip() - ) - assert tokens == [ - (Text, "Hello,\nThis is a test for "), - (Punctuation, "{{"), - (Name.Variable, "inputs"), - (Punctuation, "."), - (Name.Variable, "parameters"), - (Punctuation, "."), - (Name.Variable, "message"), - (Whitespace, " "), - (Punctuation, "}}"), - (Text, "!"), - ] - - def test_expression(self): - # TODO rewrite this test after adding support for expressions - tokens = self.lex("{{= inputs.parameters.message}}") - assert tokens == [ - (Punctuation, "{{="), - (Other, " inputs.parameters.message"), - (Punctuation, "}}"), - ] diff --git a/tests/parsers/test_parsers.py b/tests/parsers/test_parsers.py deleted file mode 100644 index c938435..0000000 --- a/tests/parsers/test_parsers.py +++ /dev/null @@ -1,80 +0,0 @@ -from unittest.mock import Mock - -from dirty_equals import IsPartialDict - -from tugboat.parsers import Node, Unexpected, parse_template, report_syntax_errors -from tugboat.parsers.ast.argo_template import Document - - -class TestParseTemplate: - def test(self): - doc = parse_template("Hello, {{ node.name }}!") - assert isinstance(doc, Document) - - -class TestReportSyntaxErrors: - - def test_no_error(self): - node = Mock( - Node, - children=[ - Mock(Node, children=[]), - Mock( - Node, - children=[ - Mock(Node, children=[]), - ], - ), - ], - ) - assert list(report_syntax_errors(node)) == [] - - def test_childless(self): - node = Mock(Node, children=[]) - assert list(report_syntax_errors(node)) == [] - - def test_errors(self): - node = Mock( - Node, - children=[ - Mock(Unexpected, text="foo", msg=None, children=[]), - Mock(Unexpected, text="bar", msg="test error", children=[]), - Mock(Unexpected, text="baz", msg=None, children=[]), - Mock(Unexpected, text="qux", msg=None, children=[]), - ], - ) - - assert list(report_syntax_errors(node)) == [ - IsPartialDict( - { - "code": "VAR001", - "loc": (), - "summary": "Syntax error", - "input": "foo", - } - ) - ] - - def test_nested_error(self): - node = Mock( - Node, - children=[ - Mock(Node, children=[]), - Mock( - Node, - children=[ - Mock(Unexpected, text="test", msg="test error", children=[]), - ], - ), - ], - ) - - assert list(report_syntax_errors(node)) == [ - { - "code": "VAR001", - "loc": (), - "summary": "Syntax error", - "msg": "Invalid syntax near 'test': test error", - "input": "test", - }, - ] diff --git a/tests/utils/test_operatoy.py b/tests/utils/test_operatoy.py index fbf9d56..56c5870 100644 --- a/tests/utils/test_operatoy.py +++ b/tests/utils/test_operatoy.py @@ -1,14 +1,7 @@ import pytest -from dirty_equals import IsPartialDict -from pydantic import BaseModel -from tugboat.references.context import ReferenceCollection from tugboat.schemas import Parameter -from tugboat.utils.operator import ( - check_model_fields_references, - find_duplicate_names, - prepend_loc, -) +from tugboat.utils.operator import find_duplicate_names, prepend_loc class TestPrependLoc: @@ -62,81 +55,3 @@ def test_picked(self): Parameter(name="name-1"), ] assert list(find_duplicate_names(items)) == [(0, "name-1"), (2, "name-1")] - - -class TestCheckModelFieldsReferences: - - def test_picked(self): - class Nested(BaseModel): - foo: str - - class Model(BaseModel): - bar: list[Nested] - baz: str - qax: int - qux: Nested - - model = Model.model_validate( - { - "baz": "leorm", - "qax": 42, - "qux": {"foo": "{{ error"}, - "bar": [ - {"foo": "bar"}, - {"foo": "{{ valid }}"}, - {"foo": "{{ invalid }}"}, - ], - } - ) - - refs = ReferenceCollection() - refs.add(("valid",)) - - assert list(check_model_fields_references(model, refs)) == [ - { - "code": "VAR002", - "fix": "{{ valid }}", - "input": "{{ invalid }}", - "loc": ("bar", 2, "foo"), - "msg": "The used reference 'invalid' is invalid.", - "summary": "Invalid reference", - "ctx": { - "reference": { - "found": ("invalid",), - "found:str": "invalid", - "closest": ("valid",), - "closest:str": "valid", - } - }, - }, - { - "code": "VAR001", - "input": "{{ error", - "loc": ("qux", "foo"), - "msg": "Invalid syntax near '{{ error': expect closing tag '}}'", - "summary": "Syntax error", - }, - ] - - def test_exclude(self): - - class Model(BaseModel): - foo: str - - model = Model.model_validate( - { - "foo": "{{ error", - } - ) - - refs = ReferenceCollection() - - assert list(check_model_fields_references(model, refs)) == [ - IsPartialDict( - { - "code": "VAR001", - "loc": ("foo",), - } - ) - ] - assert list(check_model_fields_references(model, refs, exclude=["foo"])) == [] diff --git a/tugboat/analyzers/container.py b/tugboat/analyzers/container.py index 8f7541c..961e446 100644 --- a/tugboat/analyzers/container.py +++ b/tugboat/analyzers/container.py @@ -2,10 +2,11 @@ import typing +from tugboat.analyzers.template_tag import check_template_tags_recursive from tugboat.constraints import mutually_exclusive, require_all from tugboat.core import hookimpl from tugboat.references import get_template_context -from tugboat.utils import check_model_fields_references, prepend_loc +from tugboat.utils import prepend_loc if typing.TYPE_CHECKING: from collections.abc import Iterable @@ -187,9 +188,9 @@ def check_shared_fields( } # check model fields references - for diag in check_model_fields_references(node, context.parameters): + for diag in check_template_tags_recursive(node, context.parameters): match diag["code"]: - case "VAR002": + case "VAR201": if metadata := diag.get("ctx", {}).get("reference"): ref = metadata["found:str"] diag["msg"] = ( diff --git a/tugboat/analyzers/metrics.py b/tugboat/analyzers/metrics.py index 09cb4c7..48ac75c 100644 --- a/tugboat/analyzers/metrics.py +++ b/tugboat/analyzers/metrics.py @@ -3,8 +3,8 @@ import re import typing +from tugboat.analyzers.template_tag import check_template_tags_recursive from tugboat.constraints import mutually_exclusive, require_all -from tugboat.utils import check_model_fields_references if typing.TYPE_CHECKING: from collections.abc import Iterator @@ -28,10 +28,7 @@ def check_prometheus(prometheus: Prometheus, context: Context) -> Iterator[Diagn fields=["counter", "gauge", "histogram"], require_one=True, ) - yield from check_model_fields_references( - model=prometheus, - references=context.parameters, - ) + yield from check_template_tags_recursive(prometheus, context.parameters) if prometheus.name: if not re.fullmatch(r"[a-zA-Z_][a-zA-Z0-9_]*", prometheus.name): diff --git a/tugboat/analyzers/step.py b/tugboat/analyzers/step.py index eeeab5e..d03f593 100644 --- a/tugboat/analyzers/step.py +++ b/tugboat/analyzers/step.py @@ -6,19 +6,13 @@ from rapidfuzz.process import extractOne +from tugboat.analyzers.template_tag import check_template_tags_recursive from tugboat.constraints import accept_none, mutually_exclusive, require_all from tugboat.core import get_plugin_manager, hookimpl from tugboat.references import get_step_context from tugboat.schemas import Arguments from tugboat.types import Field -from tugboat.utils import ( - check_model_fields_references, - check_value_references, - find_duplicate_names, - join_with_and, - join_with_or, - prepend_loc, -) +from tugboat.utils import find_duplicate_names, join_with_and, join_with_or, prepend_loc if typing.TYPE_CHECKING: from collections.abc import Iterable @@ -122,9 +116,9 @@ def check_argument_parameter_fields( ], ) - for diag in check_model_fields_references(param, context.parameters): + for diag in check_template_tags_recursive(param, context.parameters): match diag["code"]: - case "VAR002": + case "VAR201": diag["code"] = "STP301" if metadata := diag.get("ctx", {}).get("reference"): ref = metadata["found:str"] @@ -286,11 +280,11 @@ def check_argument_artifact_fields( # when it is wrapped, it may be a reference to a parameter or an artifact mixed_references = context.parameters + context.artifacts - for diag in prepend_loc( - ("from",), check_value_references(from_, mixed_references) + for diag in check_template_tags_recursive( + artifact, mixed_references, include=["from_"] ): match diag["code"]: - case "VAR002": + case "VAR201": diag["code"] = "STP302" if metadata := diag.get("ctx", {}).get("reference"): ref = metadata["found:str"] @@ -302,12 +296,11 @@ def check_argument_artifact_fields( # TODO fromExpression if artifact.raw: - for diag in prepend_loc( - ("raw", "data"), - check_value_references(artifact.raw.data, context.parameters), + for diag in check_template_tags_recursive( + artifact, context.parameters, include=["raw"] ): match diag["code"]: - case "VAR002": + case "VAR201": diag["code"] = "STP303" if metadata := diag.get("ctx", {}).get("reference"): ref = metadata["found:str"] @@ -458,7 +451,7 @@ def check_fields_references( step: Step, template: Template, workflow: WorkflowCompatible ) -> Iterable[Diagnosis]: ctx = get_step_context(workflow, template, step) - yield from check_model_fields_references( + yield from check_template_tags_recursive( step, ctx.parameters, exclude=["arguments", "inline", "withItems", "withSequence"], diff --git a/tugboat/analyzers/template/inputs.py b/tugboat/analyzers/template/inputs.py index ce1fc41..5c0f818 100644 --- a/tugboat/analyzers/template/inputs.py +++ b/tugboat/analyzers/template/inputs.py @@ -2,20 +2,12 @@ import typing -from tugboat.constraints import ( - accept_none, - mutually_exclusive, - require_all, -) +from tugboat.analyzers.template_tag import check_template_tags_recursive +from tugboat.constraints import accept_none, mutually_exclusive, require_all from tugboat.core import hookimpl from tugboat.references import get_workflow_context from tugboat.types import Field -from tugboat.utils import ( - check_model_fields_references, - check_value_references, - find_duplicate_names, - prepend_loc, -) +from tugboat.utils import find_duplicate_names, prepend_loc if typing.TYPE_CHECKING: from collections.abc import Iterable @@ -76,9 +68,9 @@ def _check_input_parameter( fields=["globalName"], ) - for diag in check_model_fields_references(param, context.parameters): + for diag in check_template_tags_recursive(param, context.parameters): match diag["code"]: - case "VAR002": + case "VAR201": diag["code"] = "TPL201" if metadata := diag.get("ctx", {}).get("reference"): ref = metadata["found:str"] @@ -236,12 +228,11 @@ def _check_input_artifact( # field-specific checks if artifact.raw: - for diag in prepend_loc( - ("raw", "data"), - check_value_references(artifact.raw.data, context.parameters), + for diag in check_template_tags_recursive( + artifact, context.parameters, include=["raw"] ): match diag["code"]: - case "VAR002": + case "VAR201": diag["code"] = "TPL202" if metadata := diag.get("ctx", {}).get("reference"): ref = metadata["found:str"] diff --git a/tugboat/analyzers/template/outputs.py b/tugboat/analyzers/template/outputs.py index dd738bf..52cec94 100644 --- a/tugboat/analyzers/template/outputs.py +++ b/tugboat/analyzers/template/outputs.py @@ -2,15 +2,11 @@ import typing +from tugboat.analyzers.template_tag import check_template_tags_recursive from tugboat.constraints import accept_none, mutually_exclusive, require_all from tugboat.core import hookimpl from tugboat.references import get_template_context -from tugboat.utils import ( - check_model_fields_references, - check_value_references, - find_duplicate_names, - prepend_loc, -) +from tugboat.utils import find_duplicate_names, prepend_loc if typing.TYPE_CHECKING: from collections.abc import Iterable @@ -89,9 +85,9 @@ def _check_output_parameter( # TODO check expression - for diag in check_model_fields_references(param, context.parameters): + for diag in check_template_tags_recursive(param, context.parameters): match diag["code"]: - case "VAR002": + case "VAR201": if metadata := diag.get("ctx", {}).get("reference"): ref = metadata["found:str"] diag["msg"] = ( @@ -171,11 +167,11 @@ def _check_output_artifact( ) if artifact.from_: - for diag in prepend_loc( - ("from",), check_value_references(artifact.from_, context.artifacts) + for diag in check_template_tags_recursive( + artifact, context.artifacts, include=["from_"] ): match diag["code"]: - case "VAR002": + case "VAR201": if metadata := diag.get("ctx", {}).get("reference"): ref = metadata["found:str"] diag["msg"] = ( diff --git a/tugboat/analyzers/template_tag.py b/tugboat/analyzers/template_tag.py index b5a0045..0034d07 100644 --- a/tugboat/analyzers/template_tag.py +++ b/tugboat/analyzers/template_tag.py @@ -14,11 +14,16 @@ import re import textwrap import typing +from collections.abc import Sequence import lark +from pydantic import BaseModel + +from tugboat.utils import prepend_loc if typing.TYPE_CHECKING: - from collections.abc import Iterator + from collections.abc import Iterable, Iterator + from typing import Any from lark import Token from lark.exceptions import UnexpectedInput @@ -27,6 +32,63 @@ from tugboat.types import Diagnosis +def check_template_tags_recursive( + source: Any, + references: ReferenceCollection, + *, + include: Iterable[str] = (), + exclude: Iterable[str] = (), +) -> Iterator[Diagnosis]: + """ + Check each field of the given model for errors in Argo template tags. + + Parameters + ---------- + source : Any + The input to check. + references : ReferenceCollection + The current active references. + include : Iterable[str], optional + The fields to include for checking. + exclude : Iterable[str], optional + The fields to exclude from checking. + + Yields + ------ + Diagnosis + A diagnosis for each error found. + """ + if isinstance(source, str): + yield from check_template_tags(source, references) + + elif isinstance(source, Sequence): + for idx, item in enumerate(source): + yield from prepend_loc( + (idx,), + check_template_tags_recursive( + item, references, include=include, exclude=exclude + ), + ) + + elif isinstance(source, BaseModel): + include = set(include) + exclude = set(exclude) + + for field_name, field_info in type(source).model_fields.items(): + if include and field_name not in include: + continue + if field_name in exclude: + continue + + alias = field_info.alias or field_name + value = getattr(source, field_name) + + yield from prepend_loc( + (alias,), + check_template_tags_recursive(value, references), + ) + + @functools.lru_cache(32) def parse_argo_template_tags(source: str) -> lark.Tree: """ diff --git a/tugboat/parsers/__init__.py b/tugboat/parsers/__init__.py deleted file mode 100644 index 3d7c308..0000000 --- a/tugboat/parsers/__init__.py +++ /dev/null @@ -1,106 +0,0 @@ -""" -This module provides the parser for Argo template syntax. - -It parses the input text into an abstract syntax tree (AST) that can be used to -analyze the template and generate a report. -""" - -from __future__ import annotations - -__all__ = [ - "Document", - "Node", - "SimpleReferenceTag", - "Unexpected", - "parse_template", - "report_syntax_errors", -] - -import typing - -import tugboat.parsers.ast.argo_template -import tugboat.parsers.lexer -from tugboat.parsers.ast import Document, Node, SimpleReferenceTag, Unexpected - -if typing.TYPE_CHECKING: - from collections.abc import Iterable, Iterator - - from tugboat.parsers.ast.core import Lexeme - from tugboat.types import Diagnosis - -_argo_template_lexer = None - - -def parse_template(text: str) -> Document: - """Parse Argo template text into AST.""" - global _argo_template_lexer - if not _argo_template_lexer: - _argo_template_lexer = tugboat.parsers.lexer.ArgoTemplateLexer() - - lexemes: Iterable[Lexeme] = _argo_template_lexer.get_tokens_unprocessed(text) - return tugboat.parsers.ast.argo_template.Document.parse(list(lexemes)) - - -def report_syntax_errors(node: Node) -> Iterator[Diagnosis]: - """Report errors in the AST.""" - # direct return if the node is an Unexpected node - if isinstance(node, Unexpected): - msg = f"Invalid syntax near '{node.text}'" - if node.msg: - msg += f": {node.msg}" - yield { - "code": "VAR001", - "loc": (), - "summary": "Syntax error", - "msg": msg, - "input": node.text, - } - - # iterate over the children and yield any syntax errors - i = 0 - while i < len(node.children): - # try to find consecutive `Unexpected` nodes, if any - # - # The AST builder is intend to create a single Unexpected node for each - # invalid token, so we need to group them together to provide a more - # readable error message to the user. - # - # A special case is when the `Unexpected` node has a message, we yield - # it as a single diagnosis. This is because the message is likely to be - # more informative than the concatenated text of the nodes. - unexpected_nodes = [] - while ( - i < len(node.children) - and isinstance(child := node.children[i], Unexpected) - and not child.msg - ): - unexpected_nodes.append(child) - i += 1 - - # if we found any, yield a single diagnosis - if unexpected_nodes: - text = "".join(node.text for node in unexpected_nodes) - yield { - "code": "VAR001", - "loc": (), - "summary": "Syntax error", - "msg": ( - f""" - Failed to parse the Argo workflow variable due to a syntax error. - - This error is likely caused by an invalid character or an unexpected token in the variable definition. - The syntax error is near the following text: {text} - """ - ), - "input": text, - } - # TODO - # ideally we should yield each abnormal node as a separate diagnosis - # but currently reporting does not look good, so we just return - # the first one and skip the rest. - return - - else: - # recursively check the children - yield from report_syntax_errors(node.children[i]) - i += 1 diff --git a/tugboat/parsers/ast/__init__.py b/tugboat/parsers/ast/__init__.py deleted file mode 100644 index d9fe692..0000000 --- a/tugboat/parsers/ast/__init__.py +++ /dev/null @@ -1,9 +0,0 @@ -__all__ = [ - "Document", - "Node", - "SimpleReferenceTag", - "Unexpected", -] - -from tugboat.parsers.ast.argo_template import Document, SimpleReferenceTag -from tugboat.parsers.ast.core import Node, Unexpected diff --git a/tugboat/parsers/ast/argo_template.py b/tugboat/parsers/ast/argo_template.py deleted file mode 100644 index a2377f7..0000000 --- a/tugboat/parsers/ast/argo_template.py +++ /dev/null @@ -1,174 +0,0 @@ -from __future__ import annotations - -import typing - -from tugboat._vendor.pygments.token import Name, Punctuation, Text -from tugboat.parsers.ast.core import ( - Node, - Unexpected, - consume_whitespaces, - next_lexeme_is, -) - -if typing.TYPE_CHECKING: - from collections.abc import Iterator - - from tugboat.parsers.ast.core import Lexeme - -type ReferenceTuple = tuple[str, ...] - - -class Document(Node): - - @classmethod - def parse(cls, lexemes: list[Lexeme]) -> Document: - children = [] - while lexemes: - for node_cls in [PlainText, SimpleReferenceTag, ExpressionTag, Unexpected]: - if node := node_cls.parse(lexemes): - children.append(node) - break - return cls(children=children) - - def iter_references(self) -> Iterator[tuple[SimpleReferenceTag, ReferenceTuple]]: - """ - Iterate over all references in this document. - """ - for child in self.children: - if isinstance(child, SimpleReferenceTag): - yield child, child.reference - - -class PlainText(Node): - - text: str - - @classmethod - def parse(cls, lexemes: list[Lexeme]) -> PlainText | None: - buffer = [] - while next_lexeme_is(lexemes, Text): - _, _, text = lexemes.pop(0) - buffer.append(text) - return cls(text="".join(buffer)) if buffer else None - - -class SimpleReferenceTag(Node): - - raw: str - """The raw text of the reference tag.""" - reference: ReferenceTuple - """Reference to a variable.""" - - @classmethod - def parse(cls, lexemes: list[Lexeme]) -> SimpleReferenceTag | Unexpected | None: - """ - Parse a simple reference tag. - - A typical lexemes sequence is like: - - .. code-block:: python - - [ - (0, Punctuation, "{{"), - (2, Name.Variable, "foo"), - (5, Punctuation, "."), - (6, Name.Variable, "bar"), - (9, Whitespace, " "), - (11, Punctuation, "}}"), - ] - """ - # early return - if not next_lexeme_is(lexemes, token=Punctuation, text="{{"): - return - - # consume the opening tag and whitespaces - components = [ - lexemes.pop(0), - *consume_whitespaces(lexemes), - ] - - # consume the reference - reference = [] - - if next_lexeme_is(lexemes, Name.Variable): - _, _, text = lexeme = lexemes.pop(0) - components.append(lexeme) - reference.append(text) - - while ( - True - and next_lexeme_is(lexemes, Punctuation, ".") - and next_lexeme_is(lexemes, Name.Variable, shift=1) - ): - # the dot - lexeme = lexemes.pop(0) - components.append(lexeme) - - # the name - _, _, text = lexeme = lexemes.pop(0) - components.append(lexeme) - reference.append(text) - - # consume trailing whitespaces - components += consume_whitespaces(lexemes) - - # expect the closing tag - if not next_lexeme_is(lexemes, Punctuation, "}}"): - if not lexemes or next_lexeme_is(lexemes, Text): - return Unexpected.create(components, msg="expect closing tag '}}'") - return Unexpected.create(components) - - components.append(lexemes.pop(0)) # consume the closing tag - return cls( - raw="".join(text for _, _, text in components), - reference=tuple(reference), - ) - - def __str__(self) -> str: - return self.raw - - @classmethod - def format(cls, reference: ReferenceTuple) -> str | None: - """ - Create a string representation of the reference in the same format as - this reference tag type. - """ - return "{{ " + ".".join(reference) + " }}" if reference else None - - -class ExpressionTag(Node): - """ - This class recognizes the expression tag format and stores the literal content. - """ - - literal: str - - @classmethod - def parse(cls, lexemes: list[Lexeme]) -> ExpressionTag | Unexpected | None: - # early return - if not next_lexeme_is(lexemes, token=Punctuation, text="{{="): - return - - # consume the opening tag and whitespaces - components = [ - lexemes.pop(0), - *consume_whitespaces(lexemes), - ] - - # consume all lexemes until the closing tag or the end of the lexemes - while ( - True - and lexemes - and not next_lexeme_is(lexemes, Text) - and not next_lexeme_is(lexemes, Punctuation, "}}") - ): - components.append(lexemes.pop(0)) - - # expect the closing tag - if not next_lexeme_is(lexemes, Punctuation, "}}"): - if not lexemes or next_lexeme_is(lexemes, Text): - return Unexpected.create(components, msg="expect closing tag '}}'") - return Unexpected.create(components) - - components.append(lexemes.pop(0)) # consume the closing tag - return cls(literal="".join(text for _, _, text in components)) diff --git a/tugboat/parsers/ast/core.py b/tugboat/parsers/ast/core.py deleted file mode 100644 index 958da63..0000000 --- a/tugboat/parsers/ast/core.py +++ /dev/null @@ -1,90 +0,0 @@ -from __future__ import annotations - -import typing -from abc import ABC, abstractmethod - -from pydantic import BaseModel, ConfigDict, Field - -from tugboat._vendor.pygments.token import Whitespace - -if typing.TYPE_CHECKING: - from typing import Self - - from tugboat._vendor.pygments.token import _TokenType - - type Lexeme = tuple[int, _TokenType, str] - - -class Node(ABC, BaseModel): - """Base class for AST nodes.""" - - model_config = ConfigDict(frozen=True) - - children: list[Node] = Field(default_factory=list) - - @classmethod - @abstractmethod - def parse(cls, lexemes: list[Lexeme]) -> Node | None: - """ - Parse a list of tokens into AST. This method may pop items from the list - when it consumes them. - - Parameters - ---------- - lexemes : list[Lexeme] - The list of tokens to parse. - - Returns - ------- - The AST node or None if parsing failed. - """ - - -class Unexpected(Node): - """Node to represent unparsable tokens or syntax errors.""" - - pos: int - text: str - msg: str | None = None - - @classmethod - def parse(cls, lexemes: list[Lexeme]) -> Self | None: - if not lexemes: - return - pos, _, text = lexemes.pop(0) - return cls(pos=pos, text=text) - - @classmethod - def create(cls, lexemes: list[Lexeme], msg: str | None = None) -> Self: - start_pos, _, _ = lexemes[0] - return cls( - pos=start_pos, - text="".join(text for _, _, text in lexemes), - msg=msg, - ) - - -def next_lexeme_is( - lexemes: list[Lexeme], - token: _TokenType | None = None, - text: str | None = None, - shift: int = 0, -) -> bool: - """Check if the next token in the list matches the given token or text.""" - if len(lexemes) <= 0: - return False - - _, next_token, next_text = lexemes[shift] - if token is not None and next_token not in token: - return False - if text is not None and next_text != text: - return False - return True - - -def consume_whitespaces(lexemes: list[Lexeme]) -> list[Lexeme]: - """Pop all leading whitespace tokens from the lexemes stream.""" - buffer = [] - while next_lexeme_is(lexemes, Whitespace): - buffer.append(lexemes.pop(0)) - return buffer diff --git a/tugboat/parsers/lexer.py b/tugboat/parsers/lexer.py deleted file mode 100644 index 73ea91e..0000000 --- a/tugboat/parsers/lexer.py +++ /dev/null @@ -1,41 +0,0 @@ -from __future__ import annotations - -import typing - -from tugboat._vendor.pygments.lexer import RegexLexer, words -from tugboat._vendor.pygments.token import Name, Other, Punctuation, Text, Whitespace - -if typing.TYPE_CHECKING: - from typing import ClassVar - - -class ArgoTemplateLexer(RegexLexer): - """ - Argo template language lexer - - See also - -------- - Template Tag Kinds - https://argo-workflows.readthedocs.io/en/latest/variables/#template-tag-kinds - """ - - name: ClassVar = "Argo template" - - tokens: ClassVar = { - "root": [ - (words(("{{=",)), Punctuation, "expression"), - (words(("{{",)), Punctuation, "simple-reference"), - (r"[^{]+", Text), - (r"{", Text), - ], - "simple-reference": [ - (r"\s+", Whitespace), - (r"\.", Punctuation), - (r"[\w-]+", Name.Variable), - (r"}}", Punctuation, "#pop"), - ], - "expression": [ - (r"[^}]+", Other), - (r"}}", Punctuation, "#pop"), - ], - } diff --git a/tugboat/utils/__init__.py b/tugboat/utils/__init__.py index 76a731b..2d8f3c4 100644 --- a/tugboat/utils/__init__.py +++ b/tugboat/utils/__init__.py @@ -1,19 +1,9 @@ __all__ = [ - "check_model_fields_references", - "check_value_references", "find_duplicate_names", "join_with_and", "join_with_or", "prepend_loc", ] -from tugboat.utils.humanize import ( - join_with_and, - join_with_or, -) -from tugboat.utils.operator import ( - check_model_fields_references, - check_value_references, - find_duplicate_names, - prepend_loc, -) +from tugboat.utils.humanize import join_with_and, join_with_or +from tugboat.utils.operator import find_duplicate_names, prepend_loc diff --git a/tugboat/utils/operator.py b/tugboat/utils/operator.py index 699c6d1..7970633 100644 --- a/tugboat/utils/operator.py +++ b/tugboat/utils/operator.py @@ -9,16 +9,12 @@ import typing from collections.abc import Iterable, Sequence -from pydantic import BaseModel - -from tugboat.parsers import parse_template, report_syntax_errors from tugboat.types import Diagnosis if typing.TYPE_CHECKING: - from collections.abc import Container, Iterator + from collections.abc import Iterator from typing import Protocol, Self - from tugboat.references.context import ReferenceCollection class _NamedModel(Protocol): name: str @@ -64,91 +60,3 @@ def find_duplicate_names(items: Sequence[NamedModel]) -> Iterator[tuple[int, str if len(indices) > 1: for idx in indices: yield idx, name - - -def check_value_references( - value: str, references: ReferenceCollection -) -> Iterator[Diagnosis]: - """ - Check the given value for errors that are specific to Argo workflows - variables. - - Parameters - ---------- - value : str - The value to check. - references : ReferenceCollection - The current active references. - - Yields - ------ - Diagnosis - A diagnosis for each error found. - """ - doc = parse_template(value) - yield from report_syntax_errors(doc) - - for node, ref in doc.iter_references(): - if ref in references: - continue - - ref_str = ".".join(ref) - metadata = { - "found": ref, - "found:str": ref_str, - } - - closest = references.find_closest(ref) - if closest: - metadata["closest"] = closest - metadata["closest:str"] = ".".join(closest) - - yield { - "code": "VAR002", - "loc": (), - "summary": "Invalid reference", - "msg": f"The used reference '{ref_str}' is invalid.", - "input": str(node), - "fix": node.format(closest), - "ctx": {"reference": metadata}, - } - - -def check_model_fields_references( - model: BaseModel, references: ReferenceCollection, *, exclude: Container[str] = () -) -> Iterator[Diagnosis]: - """ - Check the fields of the given model for errors that are specific to Argo - workflows variables. - - Parameters - ---------- - model : BaseModel - The model to check. This function will check all fields of the model - that are of type str. - references : ReferenceCollection - The current active references. - exclude : Container[str] - The fields to exclude from the check. - - Yields - ------ - Diagnosis - A diagnosis for each error found. - """ - - def _check(item): - if isinstance(item, str): - yield from check_value_references(item, references) - - elif isinstance(item, BaseModel): - for field, value in item: - yield from prepend_loc((field,), _check(value)) - - elif isinstance(item, Sequence): - for idx, child in enumerate(item): - yield from prepend_loc((idx,), _check(child)) - - for field, value in model: - if field not in exclude: - yield from prepend_loc((field,), _check(value))