Address review comments

Author: hvitved

rust/ql/consistency-queries/PathResolutionConsistency.ql

@@ -0,0 +1,8 @@
+/**
+ * @name Path resolution inconsistencies
+ * @description Lists the path resolution inconsistencies in the database.  This query is intended for internal use.
+ * @kind table
+ * @id rust/diagnostics/path-resolution-consistency
+ */
+
+import codeql.rust.internal.PathResolutionConsistency

rust/ql/consistency-queries/TypeInferenceConsistency.ql

@@ -0,0 +1,8 @@
+/**
+ * @name Type inference inconsistencies
+ * @description Lists the type inference inconsistencies in the database.  This query is intended for internal use.
+ * @kind table
+ * @id rust/diagnostics/type-inference-consistency
+ */
+
+import codeql.rust.internal.TypeInferenceConsistency

rust/ql/lib/codeql/rust/elements/internal/CallExprBaseImpl.qll

@@ -27,6 +27,6 @@ module Impl {
    */
   class CallExprBase extends Generated::CallExprBase {
     /** Gets the static target of this call, if any. */
-    Callable getStaticTarget() { none() }
+    Callable getStaticTarget() { none() } // overridden by subclasses
   }
 }

rust/ql/lib/codeql/rust/elements/internal/GenericArgListImpl.qll

@@ -26,7 +26,7 @@ module Impl {
     override string toAbbreviatedString() { result = "<...>" }
 
     /** Gets the `i`th type argument of this list. */
-    TypeRepr getTypeArgument(int i) {
+    TypeRepr getTypeArg(int i) {
       result =
         rank[i + 1](TypeRepr res, int j |
           res = this.getGenericArg(j).(TypeArg).getTypeRepr()
@@ -36,6 +36,6 @@ module Impl {
     }
 
     /** Gets a type argument of this list. */
-    TypeRepr getATypeArgument() { result = this.getTypeArgument(_) }
+    TypeRepr getATypeArg() { result = this.getTypeArg(_) }
   }
 }

rust/ql/lib/codeql/rust/elements/internal/PathResolution.qll

@@ -317,49 +317,53 @@ class ImplItemNode extends ImplOrTraitItemNode instanceof Impl {
   }
 
   /**
-   * Holds if this `impl` block is constrained. Examples:
+   * Holds if this `impl` block is not fully parametric. That is, the implementing
+   * type is generic and the implementation is not parametrically polymorphic in all
+   * the implementing type's arguments.
+   *
+   * Examples:
    *
    * ```rust
-   * impl Foo { ... } // unconstrained
+   * impl Foo { ... } // fully parametric
    *
-   * impl<T> Foo<T> { ... } // unconstrained
+   * impl<T> Foo<T> { ... } // fully parametric
    *
-   * impl Foo<i64> { ... } // constrained
+   * impl Foo<i64> { ... } // not fully parametric
    *
-   * impl<T> Foo<Foo<T>> { ... } // constrained
+   * impl<T> Foo<Foo<T>> { ... } // not fully parametric
    *
-   * impl<T: Trait> Foo<T> { ... } // constrained
+   * impl<T: Trait> Foo<T> { ... } // not fully parametric
    *
-   * impl<T> Foo<T> where T: Trait { ... } // constrained
+   * impl<T> Foo<T> where T: Trait { ... } // not fully parametric
    * ```
    */
   pragma[nomagic]
-  predicate isConstrained() {
+  predicate isNotFullyParametric() {
     exists(TypeRepr arg | arg = this.getASelfTyArg() |
       not exists(this.getASelfTyTypeParamArg(arg))
       or
-      this.getASelfTyTypeParamArg(arg).isConstrained()
+      this.getASelfTyTypeParamArg(arg).hasTraitBound()
     )
   }
 
   /**
-   * Holds if this `impl` block is unconstrained. Examples:
+   * Holds if this `impl` block is fully parametric. Examples:
    *
    * ```rust
-   * impl Foo { ... } // unconstrained
+   * impl Foo { ... } // fully parametric
    *
-   * impl<T> Foo<T> { ... } // unconstrained
+   * impl<T> Foo<T> { ... } // fully parametric
    *
-   * impl Foo<i64> { ... } // constrained
+   * impl Foo<i64> { ... } // not fully parametric
    *
-   * impl<T> Foo<Foo<T>> { ... } // constrained
+   * impl<T> Foo<Foo<T>> { ... } // not fully parametric
    *
-   * impl<T: Trait> Foo<T> { ... } // constrained
+   * impl<T: Trait> Foo<T> { ... } // not fully parametric
    *
-   * impl<T> Foo<T> where T: Trait { ... } // constrained
+   * impl<T> Foo<T> where T: Trait { ... } // not fully parametric
    * ```
    */
-  predicate isUnconstrained() { not this.isConstrained() }
+  predicate isFullyParametric() { not this.isNotFullyParametric() }
 
   override AssocItemNode getAnAssocItem() { result = super.getAssocItemList().getAnAssocItem() }
 
@@ -481,18 +485,18 @@ private class TypeParamItemNode extends ItemNode instanceof TypeParam {
   ItemNode resolveABound() { result = resolvePath(this.getABoundPath()) }
 
   /**
-   * Holds if this type parameter is constrained. Examples:
+   * Holds if this type parameter has a trait bound. Examples:
    *
    * ```rust
-   * impl<T> Foo<T> { ... } // unconstrained
+   * impl<T> Foo<T> { ... } // has no trait bound
    *
-   * impl<T: Trait> Foo<T> { ... } // constrained
+   * impl<T: Trait> Foo<T> { ... } // has trait bound
    *
-   * impl<T> Foo<T> where T: Trait { ... } // constrained
+   * impl<T> Foo<T> where T: Trait { ... } // has trait bound
    * ```
    */
   pragma[nomagic]
-  predicate isConstrained() {
+  predicate hasTraitBound() {
     exists(this.getABoundPath())
     or
     exists(ItemNode declaringItem, WherePred wp |
@@ -503,18 +507,18 @@ private class TypeParamItemNode extends ItemNode instanceof TypeParam {
   }
 
   /**
-   * Holds if this type parameter is unconstrained. Examples:
+   * Holds if this type parameter has no trait bound. Examples:
    *
    * ```rust
-   * impl<T> Foo<T> { ... } // unconstrained
+   * impl<T> Foo<T> { ... } // has no trait bound
    *
-   * impl<T: Trait> Foo<T> { ... } // constrained
+   * impl<T: Trait> Foo<T> { ... } // has trait bound
    *
-   * impl<T> Foo<T> where T: Trait { ... } // constrained
+   * impl<T> Foo<T> where T: Trait { ... } // has trait bound
    * ```
    */
   pragma[nomagic]
-  predicate isUnconstrained() { not this.isConstrained() }
+  predicate hasNoTraitBound() { not this.hasTraitBound() }
 
   override string getName() { result = TypeParam.super.getName().getText() }
 

rust/ql/lib/codeql/rust/elements/internal/Type.qll

@@ -18,7 +18,12 @@ newtype TType =
   TRefTypeParameter() or
   TSelfTypeParameter()
 
-/** A type without type arguments. */
+/**
+ * A type without type arguments.
+ *
+ * Note that this type includes things that, strictly speaking, are not Rust
+ * types, such as traits and implementation blocks.
+ */
 abstract class Type extends TType {
   /** Gets the method `name` belonging to this type, if any. */
   pragma[nomagic]
@@ -72,13 +77,13 @@ abstract private class StructOrEnumType extends Type {
     exists(ImplOrTraitItemNode impl | result = impl.getAnAssocItem() |
       impl instanceof Trait
       or
-      impl.(ImplItemNode).isUnconstrained()
+      impl.(ImplItemNode).isFullyParametric()
     )
   }
 
   final override ImplMention getABaseTypeMention() {
     this.asItemNode() = result.resolveSelfTy() and
-    result.isUnconstrained()
+    result.isFullyParametric()
   }
 }
 

rust/ql/lib/codeql/rust/elements/internal/TypeInference.qll

@@ -22,7 +22,7 @@ private module Input1 implements InputSig1<Location> {
     // method type parameters are matched by position instead of by type
     // parameter entity, to avoid extra recursion through method call resolution
     TMethodTypeArgumentPosition(int pos) {
-      exists(any(MethodCallExpr mce).getGenericArgList().getTypeArgument(pos))
+      exists(any(MethodCallExpr mce).getGenericArgList().getTypeArg(pos))
     } or
     TTypeParamTypeArgumentPosition(TypeParam tp)
 
@@ -124,11 +124,13 @@ private Type inferAnnotatedType(AstNode n, TypePath path) {
 }
 
 /**
- * Holds if the type of `n1` at `path1` is the same as the type of `n2` at `path2`.
+ * Holds if the type of `n1` at `path1` is the same as the type of `n2` at
+ * `path2` and type information should propagate in both directions through the
+ * type equality.
  */
 bindingset[path1]
 bindingset[path2]
-private predicate typeSymmetry(AstNode n1, TypePath path1, AstNode n2, TypePath path2) {
+private predicate typeEquality(AstNode n1, TypePath path1, AstNode n2, TypePath path2) {
   exists(Variable v |
     path1 = path2 and
     n1 = v.getAnAccess()
@@ -159,11 +161,11 @@ private predicate typeSymmetry(AstNode n1, TypePath path1, AstNode n2, TypePath
 }
 
 pragma[nomagic]
-private Type inferTypeSymmetry(AstNode n, TypePath path) {
+private Type inferTypeEquality(AstNode n, TypePath path) {
   exists(AstNode n2, TypePath path2 | result = inferType(n2, path2) |
-    typeSymmetry(n, path, n2, path2)
+    typeEquality(n, path, n2, path2)
     or
-    typeSymmetry(n2, path2, n, path)
+    typeEquality(n2, path2, n, path)
   )
 }
 
@@ -222,7 +224,7 @@ private Type inferImplicitSelfType(SelfParam self, TypePath path) {
 
 private TypeMention getExplicitTypeArgMention(Path path, TypeParam tp) {
   exists(int i |
-    result = path.getPart().getGenericArgList().getTypeArgument(pragma[only_bind_into](i)) and
+    result = path.getPart().getGenericArgList().getTypeArg(pragma[only_bind_into](i)) and
     tp = resolvePath(path).getTypeParam(pragma[only_bind_into](i))
   )
   or
@@ -316,7 +318,7 @@ private module RecordExprMatchingInput implements MatchingInputSig {
   class AccessPosition = DeclarationPosition;
 
   class Access extends RecordExpr {
-    Type getExplicitTypeArgument(TypeArgumentPosition apos, TypePath path) {
+    Type getTypeArgument(TypeArgumentPosition apos, TypePath path) {
       result = getExplicitTypeArgMention(this.getPath(), apos.asTypeParam()).resolveTypeAt(path)
     }
 
@@ -535,10 +537,10 @@ private module CallExprBaseMatchingInput implements MatchingInputSig {
 
   class Access extends CallExprBase {
     private TypeReprMention getMethodTypeArg(int i) {
-      result = this.(MethodCallExpr).getGenericArgList().getTypeArgument(i)
+      result = this.(MethodCallExpr).getGenericArgList().getTypeArg(i)
     }
 
-    Type getExplicitTypeArgument(TypeArgumentPosition apos, TypePath path) {
+    Type getTypeArgument(TypeArgumentPosition apos, TypePath path) {
       exists(TypeMention arg | result = arg.resolveTypeAt(path) |
         arg = getExplicitTypeArgMention(CallExprImpl::getFunctionPath(this), apos.asTypeParam())
         or
@@ -616,7 +618,7 @@ private module CallExprBaseMatchingInput implements MatchingInputSig {
             pathAdj = TypePath::singleton(TRefTypeParameter()) and
             tAdj = t
           else
-            if path.startsWith(TRefTypeParameter(), _)
+            if path.isCons(TRefTypeParameter(), _)
             then
               pathAdj = path and
               tAdj = t
@@ -628,10 +630,10 @@ private module CallExprBaseMatchingInput implements MatchingInputSig {
             )
         else (
           // adjust for implicit deref
-          path.startsWith(TRefTypeParameter(), pathAdj) and
+          path.isCons(TRefTypeParameter(), pathAdj) and
           tAdj = t
           or
-          not path.startsWith(TRefTypeParameter(), _) and
+          not path.isCons(TRefTypeParameter(), _) and
           not (t = TRefType() and path.isEmpty()) and
           pathAdj = path and
           tAdj = t
@@ -674,19 +676,19 @@ private Type inferCallExprBaseType(AstNode n, TypePath path) {
         if receiverType = TRefType()
         then
           path = path0 and
-          path0.startsWith(TRefTypeParameter(), _)
+          path0.isCons(TRefTypeParameter(), _)
           or
           // adjust for implicit deref
-          not path0.startsWith(TRefTypeParameter(), _) and
+          not path0.isCons(TRefTypeParameter(), _) and
           not (path0.isEmpty() and result = TRefType()) and
           path = TypePath::cons(TRefTypeParameter(), path0)
         else (
-          not path0.startsWith(TRefTypeParameter(), _) and
+          not path0.isCons(TRefTypeParameter(), _) and
           not (path0.isEmpty() and result = TRefType()) and
           path = path0
           or
           // adjust for implicit borrow
-          path0.startsWith(TRefTypeParameter(), path)
+          path0.isCons(TRefTypeParameter(), path)
         )
       )
     else path = path0
@@ -748,7 +750,7 @@ private module FieldExprMatchingInput implements MatchingInputSig {
   class AccessPosition = DeclarationPosition;
 
   class Access extends FieldExpr {
-    Type getExplicitTypeArgument(TypeArgumentPosition apos, TypePath path) { none() }
+    Type getTypeArgument(TypeArgumentPosition apos, TypePath path) { none() }
 
     AstNode getNodeAt(AccessPosition apos) {
       result = this.getExpr() and
@@ -781,10 +783,10 @@ private module FieldExprMatchingInput implements MatchingInputSig {
     if apos.isSelf()
     then
       // adjust for implicit deref
-      path.startsWith(TRefTypeParameter(), pathAdj) and
+      path.isCons(TRefTypeParameter(), pathAdj) and
       tAdj = t
       or
-      not path.startsWith(TRefTypeParameter(), _) and
+      not path.isCons(TRefTypeParameter(), _) and
       not (t = TRefType() and path.isEmpty()) and
       pathAdj = path and
       tAdj = t
@@ -824,7 +826,7 @@ private Type inferFieldExprType(AstNode n, TypePath path) {
         if receiverType = TRefType()
         then
           // adjust for implicit deref
-          not path0.startsWith(TRefTypeParameter(), _) and
+          not path0.isCons(TRefTypeParameter(), _) and
           not (path0.isEmpty() and result = TRefType()) and
           path = TypePath::cons(TRefTypeParameter(), path0)
         else path = path0
@@ -846,7 +848,7 @@ private Type inferRefExprType(Expr e, TypePath path) {
     or
     e = re and
     exists(TypePath exprPath | result = inferType(re.getExpr(), exprPath) |
-      if exprPath.startsWith(TRefTypeParameter(), _)
+      if exprPath.isCons(TRefTypeParameter(), _)
       then
         // `&x` simply means `x` when `x` already has reference type
         path = exprPath
@@ -975,7 +977,7 @@ private module Cached {
     Stages::TypeInference::backref() and
     result = inferAnnotatedType(n, path)
     or
-    result = inferTypeSymmetry(n, path)
+    result = inferTypeEquality(n, path)
     or
     result = inferImplicitSelfType(n, path)
     or

rust/ql/lib/codeql/rust/elements/internal/TypeMention.qll

@@ -78,7 +78,7 @@ class TypeReprMention extends TypeMention, TypeRepr {
 
 class PathMention extends TypeMention, Path {
   override TypeMention getTypeArgument(int i) {
-    result = this.getPart().getGenericArgList().getTypeArgument(i)
+    result = this.getPart().getGenericArgList().getTypeArg(i)
     or
     // `Self` paths inside traits and `impl` blocks have implicit type arguments
     // that are the type parameters of the trait or impl. For example, in
@@ -93,7 +93,7 @@ class PathMention extends TypeMention, Path {
     //
     // the `Self` return type is shorthand for `Foo<T>`.
     exists(ImplOrTraitItemNode node | this = node.getASelfPath() |
-      result = node.(ImplItemNode).getSelfPath().getPart().getGenericArgList().getTypeArgument(i)
+      result = node.(ImplItemNode).getSelfPath().getPart().getGenericArgList().getTypeArg(i)
       or
       result = node.(Trait).getGenericParamList().getTypeParam(i)
     )
@@ -140,7 +140,7 @@ private predicate isImplSelfTypeParam(
 ) {
   exists(PathMention path |
     selfPath = impl.getSelfPath() and
-    path = selfPath.getPart().getGenericArgList().getTypeArgument(i).(PathTypeRepr).getPath() and
+    path = selfPath.getPart().getGenericArgList().getTypeArg(i).(PathTypeRepr).getPath() and
     tp = path.resolveType()
   )
 }