Linter plugin via Trustfall

[Boshen]

Note

Update: We decided to stop exploring this approach and is going to try with AST transfer instead.

From @u9g based on https://github.com/obi1kenobi/trustfall. The query is a subset of GraphQL.

Examples:

BAD_ASSIGN_IN_CONDITION from deepscan

query {
  File {
    ast_node {
      ... on IfStatementOrTernary {
        test {
            # unwraps parens, because there will always be parens around a ternary with an assignment otherwise
            # it's a constant condition
            # NOTE: If we want to allow paren'd if statements, we'd have to split this into two queries,
            # one for IfStatement and one for Ternary
            inner_expr {
              ... on AssignmentExpression {
                span {
                  span_start: start @output
                  span_end: end @output
                }
              }
            }
          }
        }
      }
    }
  }
}

BAD_BITWISE_OPERATOR from deepscan

query {
  File {
    # x & x.???
    ast_node {
      ... on BinaryExpression {
        left {
          ... on Identifier {
            name @tag
          }
        }
        operator {
          is_bitwise @filter(op: "=", value: ["$true"])
        }
        right {
          ... on FieldAccessOnObject {
            final_assignment {
              ... on Identifier {
                name @filter(op: "=", value: ["%name"])
              }
            }
          }
        }

        span {
          span_start: start @output
          span_end: end @output
        }
      }
    }
  }
}

query {
  File {
    # x |= ??? will be flagged when
    # the ??? isn't a NumberLiteral ie... 1, 2, 19
    # the ??? isn't 1 << 19 or 1 >> 19 or any bitwise operation where both sides are numbers
    ast_node {
      ... on AssignmentExpression {
        operator {
          is_bitwise @filter(op: "=", value: ["$true"])
        }

        # this basically says don't allow any expressions where the right is something like `19`, or any number
        right @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) {
          ... on NumberLiteral {
            # needed here because we can't have a no field used coercion,
            # but we don't actually need span->start
            span {
              start
            }
          }
        }

        # this says don't allow any expressions where the right is something like `1 << 12`, `1 | 12`, or any bitwise expression
        right @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) {
          ... on BinaryExpression {
            operator {
              is_bitwise @filter(op: "=", value: ["$true"])
            }
            both_sides @fold @transform(op: "count") @filter(op: "=", value: ["$two"]) {
              ... on NumberLiteral {
                # needed here because we can't have a no field used coercion,
                # but we don't actually need span->start
                span {
                  start
                }
              }
            }
          }
        }

        span {
          span_start: start @output
          span_end: end @output
        }
      }
    }
  }
}

no_async_promise_execs from eslint

query {
  File {
    ast_node {
      ... on NewExpression {
        callee {
          ... on Identifier {
            name @filter(op: "=", value: ["$Promise"])
          }
        }

        arguments {
          ... on ExpressionArgument {
            inner_expr {
              ... on Function {
                is_async @filter(op: "=", value: ["$true"])
                
                span {
                  span_start: start @output
                  span_end: end @output
                }
              } 
            }
          }
        }
      }
    }
  }
}

code-declare-service-brand

query {
  File {
    ast_node {
      ... on ClassPropertyDefinition {
        key {
          ... on IdentifierPropertyKey {
            identifier @filter(op: "=", value: ["$_serviceBrand"])
          }
        }

        value @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) {
          ... on Identifier {
            name @filter(op: "=", value: ["$undefined"])
          }
        }

        span_: span {
          start @output
          end @output
        }
      }
    }
  }
}

code-no-look-behind-regex

query {
  File {
    ast_node {
      ... on RegExp { # handles new RegExp("hello") and /hello/
        pattern @filter(op: "regex", value: ["$look_behind_regex"])

        span_: span {
          start @output
          end @output
        }
      }
    }
  }
}

args:

Arguments(
  args: {
      "look_behind_regex": String("\\(\\?<!.+|\\(\\?<=.+")
    }
)

vscode-dts-literal-or-types

query {
  File {
    ast_node {
      ... on TSTypeAnnotation {
        ts_type {
          ... on TSUnionType {

            subtypes @fold @transform(op: "count") @tag(name: "string_subtype_count") {
              ... on TSLiteralType {
                expr {
                  ... on StringExpression {
                    span {
                      start # don't need the span but need to say we need some part of StringExpression
                    }
                  }
                }
              }
            }

            # if we have the same number of subtypes as string subtypes, then all subtypes are strings
            subtypes @fold @transform(op: "count") @filter(op: "=", value: ["%string_subtype_count"]) {
              span {
                start # don't need the span but need to say we need some part of the TSType
              }
            }

            span_: span {
              start @output
              end @output
            }
          }
        }
      }
    }
  }
}

code-no-test-only

query {
  File {
    ast_node {
      ... on CallExpression {
        callee {
          ... on FieldAccessOnObject {
            property_name {
              name @filter(op: "=", value: ["$only"])
              span_: span {
                start @output
                end @output
              }
            }
            called_on {
              ... on Identifier {
                name @filter(op: "regex", value: ["$test_or_suite_regex"])
              }
            }
          }
        }
      }
    }
  }
}

args:

Arguments(
  args: {
      "test_or_suite_regex": String("^(test|suite)$"),
      "only": String("only")
  }
)

vscode-dts-cancellation

query {
  File {
    ast_node {
      ... on TSInterfaceDeclaration {
        name @filter(op: "has_suffix", value: ["$Provider"])
        body {
          ... on TSMethodSignature {
            key {
              ... on IdentifierPropertyKey {
                identifier @filter(op: "regex", value: ["$method_name_regex"]) # /(provide|resolve).+/
              }
            }

            parameters @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) {
              ... on IdentifierParameter {
                name @filter(op: "=", value: ["$token"])
              }
            }

            span_: span {
              start @output
              end @output
            }
          }
        }
      }
    }
  }
}

args:

Arguments(
  args: {
      "Provider": String("Provider"),
      "zero": Uint64(0),
      "method_name_regex": String("(provide|resolve).+"),
      "token": String("token")
  }
)

vscode-dts-string-type-literals

query {
  File {
    ast_node {
      ... on TSPropertySignature {
        is_optional @filter(op: "=", value: ["$false"])

        type_annotation {
          ts_type {
            ... on TSUnionType {

                subtypes @fold @transform(op: "count") @tag(name: "string_subtype_count") {
                ... on TSLiteralType {
                  expr {
                    ... on StringExpression {
                      span {
                        start # don't need the span but need to say we need some part of StringExpression
                      }
                    }
                  }
                }
              }

              # if we have the same number of subtypes as string subtypes, then all subtypes are strings
              subtypes @fold @transform(op: "count") @filter(op: "=", value: ["%string_subtype_count"]) {
                span {
                  start # don't need the span but need to say we need some part of the TSType
                }
              }

              span_: span {
                start @output
                end @output
              }
            }
          }
        }
      }
    }
  }
}

vscode-dts-use-thenable

query {
  File {
    ast_node {
      ... on TSTypeAnnotation {
        ts_type {
          ... on TSTypeReference {
            name {
              ... on TSIdentifierReference {
                name @filter(op: "=", value: ["$Promise"])
                span_: span {
                  start @output
                  end @output
                }
              }
            }
          }
        }
      }
    }
  }
}

vscode-dts-create-func

query {
  File {
    ast_node {
      ... on FunctionSignature {
        name @filter(op: "has_prefix", value: ["$create"])
        return_type {
          ts_type {
            ... on TSTypeReference {
              name {
                ... on TSIdentifierReference {
                  name @filter(op: "regex", value: ["$promise_or_thenable_regex"]) # /Promise|Thenable/

                  span_: span {
                    start @output
                    end @output
                  }
                }
              }
            }
          }
        }
      }
    }
  }
}

args:

Arguments(
  args: {
      "create": String("create"),
      "promise_or_thenable_regex": String("Promise|Thenable")
    }
)

vscode-dts-provider-naming

query {
  File {
    ast_node {
      ... on TSInterfaceDeclaration {
        name @filter(op: "not_one_of", value: ["$allowed_providers"])
             @filter(op: "has_suffix", value: ["$Provider"])
        body {
          ... on TSMethodSignature {
            name @filter(op: "not_has_prefix", value: ["$provide"])
                 @filter(op: "not_has_prefix", value: ["$resolve"])
                 @filter(op: "not_has_prefix", value: ["$prepare"])
            span_: span {
              start @output
              end @output
            }
          }
        }
      }
    }
  }
}

args:

Arguments(
  args: {
      "allowed_providers": List([
        String("AllowedProvider")
      ]),
      "Provider": String("Provider"),
      "provide": String("provide"),
      "resolve": String("resolve"),
      "prepare": String("prepare"),
  },
)

vscode-dts-interface-naming

query {
  File {
    ast_node {
      ... on TSInterfaceDeclaration {
        name @filter(op: "not_has_prefix", value: ["$I"])
        span_: span {
          start @output
          end @output
        }
      }
    }
  }
}

To note:

1. the entire design of the class/inheritance hierarchy is completely decoupled from the code and thus changeable in ways that the same rule in rust can't be
2. if the syntax isn't great we can decide on a different frontend and/or build on top of trustfall as it's opensource to add our own things to it as needed (although it's already very customizable now, it's like needing to customize postgres's sql)

[u9g]

Should this go directly into oxc_linter or a new crate?

[u9g]

Do we want to support multifile queries in a single query?

subquestion: do we want to support also querying the package.json?

[Boshen]

Idea: Oxc as a background service, where trustfall queries into it. Or some kind of reverse dependency infrastructure - This plugin system dependents on Oxc.

[u9g]

Those pictures don't really do it justice. Here are some queries with descriptions. Feel free to ask any questions.

BAD_ASSIGN_IN_CONDITION from deepscan

query {
  File {
    ast_node {
      ... on IfStatementOrTernary {
        test {
            # unwraps parens, because there will always be parens around a ternary with an assignment otherwise
            # it's a constant condition
            # NOTE: If we want to allow paren'd if statements, we'd have to split this into two queries,
            # one for IfStatement and one for Ternary
            inner_expr {
              ... on AssignmentExpression {
                span {
                  span_start: start @output
                  span_end: end @output
                }
              }
            }
          }
        }
      }
    }
  }
}

BAD_BITWISE_OPERATOR from deepscan

query {
  File {
    # x & x.???
    ast_node {
      ... on BinaryExpression {
        left {
          ... on Identifier {
            name @tag
          }
        }
        operator {
          is_bitwise @filter(op: "=", value: ["$true"])
        }
        right {
          ... on FieldAccessOnObject {
            final_assignment {
              ... on Identifier {
                name @filter(op: "=", value: ["%name"])
              }
            }
          }
        }

        span {
          span_start: start @output
          span_end: end @output
        }
      }
    }
  }
}

query {
  File {
    # x |= ??? will be flagged when
    # the ??? isn't a NumberLiteral ie... 1, 2, 19
    # the ??? isn't 1 << 19 or 1 >> 19 or any bitwise operation where both sides are numbers
    ast_node {
      ... on AssignmentExpression {
        operator {
          is_bitwise @filter(op: "=", value: ["$true"])
        }

        # this basically says don't allow any expressions where the right is something like `19`, or any number
        right @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) {
          ... on NumberLiteral {
            # needed here because we can't have a no field used coercion,
            # but we don't actually need span->start
            span {
              start
            }
          }
        }

        # this says don't allow any expressions where the right is something like `1 << 12`, `1 | 12`, or any bitwise expression
        right @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) {
          ... on BinaryExpression {
            operator {
              is_bitwise @filter(op: "=", value: ["$true"])
            }
            both_sides @fold @transform(op: "count") @filter(op: "=", value: ["$two"]) {
              ... on NumberLiteral {
                # needed here because we can't have a no field used coercion,
                # but we don't actually need span->start
                span {
                  start
                }
              }
            }
          }
        }

        span {
          span_start: start @output
          span_end: end @output
        }
      }
    }
  }
}

no_async_promise_execs from eslint

query {
  File {
    ast_node {
      ... on NewExpression {
        callee {
          ... on Identifier {
            name @filter(op: "=", value: ["$Promise"])
          }
        }

        arguments {
          ... on ExpressionArgument {
            inner_expr {
              ... on Function {
                is_async @filter(op: "=", value: ["$true"])
                
                span {
                  span_start: start @output
                  span_end: end @output
                }
              } 
            }
          }
        }
      }
    }
  }
}

[Boshen]

@u9g How much performance overhead are we adding here?

[u9g]

I haven't done extensive benchmarking but everything is already lazily evaluated so we never do more work then we need to. In addition, if need be, there is an extensive optimization api which allows for converting O(n) queries down to O(1). See for more info on what that could look like: https://predr.ag/blog/speeding-up-rust-semver-checking-by-over-2000x/

As of now though, it's a very minimal addition in time compared to native queries, we are talking in the less than 10ms range from what I've seen implementing it into oxc now, and I think most of that is overhead from elsewhere.

[Boshen]

@u9g Can you do a quick survey and see if these typical eslint plugins can be ported to Trustfall. If so, how much effort for writing these plugins, and how much effort for supporting all the features?

https://github.com/microsoft/vscode/tree/main/.eslintplugin

[obi1kenobi]

If you need to name a field somewhere just to satisfy the parser, you can always use __typename. It's always available in every scope and you won't ever be in a situation where you need to expand an extra edge just because the original vertex had no properties.

[u9g]

Is it available in the case that I don't implement __typename? or will that throw?

[obi1kenobi]

You should always implement __typename, or else you're signing up yourself or someone else for a confusing debugging session later.

You can get a free __typename implementation if you use the TrustfallEnumVertex derive macro or the resolve_typename helper method.

It is undefined whether evaluating a query that names unimplemented fields works fine, errors, panics, or does something else entirely.

[u9g]

Is there any reason I shouldn't just return the type_name that I get in the resolve_property call?

[obi1kenobi]

Because it might not be correct — consider situations where the query knows only a supertype (e.g. ASTNode) of the current vertex but you're at a specific vertex type like FunctionDefinition. Take a look at the resolve_typename helper to see a way to safely do a variant of what you proposed.

[Boshen]

Case for no-restricted-syntax

https://github.com/vuejs/core/blob/9f8e98af891f456cc8cc9019a31704e5534d1f08/.eslintrc.cjs#L21-L30

[obi1kenobi]

Those should be super easy to do. I don't know the exact schema that @u9g is using but the lints should be easy for any schema in the domain.

[u9g]

It's not super easy to do because of how complex the variable declaration syntax is.

I will definitely be exploring how we can simplify the schema, but in their current form (that is working!), this is what we have.

For the no rest element rule, It takes two queries as of now:

query {
  File {
    ast_node {
      ... on VariableDeclarations {
        declarations {
          left {
            ... on ArrayDestructuringAssignment {
              rest {
                span_: span {
                  start @output
                  end @output
                }
              }
            }
          }
        }
      }
    }
  }
}

query {
  File {
    ast_node {
      ... on VariableDeclarations {
        declarations {
          left {
            ... on ObjectDestructingAssignment {
              rest {
                span_: span {
                  start @output
                  end @output
                }
              }
            }
          }
        }
      }
    }
  }
}

For the no object spreads rule, it looks like this:

query {
  File {
    ast_node {
      ... on ObjectExpression {
        properties {
          ... on SpreadObjectProperty {
            span_: span {
              start @output
              end @output
            }
          }
        }
      }
    }
  }
}

For the no await rule:

query {
  File {
    ast_node {
      ... on AwaitExpression {
        span_: span {
          start @output
          end @output
        }
      }
    }
  }
}

[obi1kenobi]

I guess I'm too used to cargo-semver-checks' queries, so I was like "no tags, no recursion, not even a filter, that seems really simple" 😅

Some ideas for simplifying the schema (not all of these are good! use caution and good judgment):

• Perhaps a common DestructuringAssignment interface could be extracted to unify the first two queries?
• Sometimes it makes sense to split patterns that look like "generic edge -> type coercion" into separate edges each with a specific type. For example, instead of doing { File { ast_node { ... on FunctionDeclaration { ... } } } } it could make sense to have { File { function_declaration { ... } } } either instead of or (more likely) in addition to the ast_node edge.