Repository: geldata/gel
Pull request: Forbid reference to link properties not from links
closes #1471

Problem statement:
InternalServerError: 'NoneType' object has no attribute 'ptrref'
- EdgeDB Version: 1.0-alpha.3+dev.247.g04b74247

Steps to Reproduce:
Error occurs when a computed property has a link property path on a subquery, eg.

```
WITH MODULE schema
SELECT ObjectType {
  name,

  # error occurs with this line:
  annotationValues := (SELECT .annotations)@value

  # these are ok:
  # annotationValues := .annotations@value
  # annotationNames := (SELECT .annotations).name
  # annotationNames := .annotations.name
  
} FILTER EXISTS .annotations
```

Hints from issue discussion:
`(SELECT .annotations)@value` should be a `ReferenceError`.  The only valid form of link property reference is `.link@prop`.

Commit message: Forbid reference to link properties not from links

Patch to review:
diff --git a/edb/edgeql/compiler/setgen.py b/edb/edgeql/compiler/setgen.py
index 6f46dc9acbe..8b8cb16b2db 100644
--- a/edb/edgeql/compiler/setgen.py
+++ b/edb/edgeql/compiler/setgen.py
@@ -264,6 +264,15 @@ def compile_path(expr: qlast.Path, *, ctx: context.ContextLevel) -> irast.Set:
             if ptr_expr.type == 'property':
                 # Link property reference; the source is the
                 # link immediately preceding this step in the path.
+                if path_tip.rptr is None:
+                    source = get_set_type(path_tip, ctx=ctx)
+                    s_dn = source.get_displayname(ctx.env.schema)
+                    raise errors.InvalidReferenceError(
+                        f"link property {ptr_name!r} "
+                        f"should be accessed from link, not from {s_dn!r}",
+                        context=ptr_expr.ptr.context,
+                    )
+
                 if isinstance(path_tip.rptr.ptrref,
                               irast.TypeIntersectionPointerRef):
                     ind_prefix, ptrs = typegen.collapse_type_intersection_rptr(
diff --git a/tests/test_edgeql_select.py b/tests/test_edgeql_select.py
index e3be117ba32..7bfc1debd77 100644
--- a/tests/test_edgeql_select.py
+++ b/tests/test_edgeql_select.py
@@ -4884,6 +4884,34 @@ async def test_edgeql_select_linkproperty_03(self):
             }],
         )
 
+    async def test_edgeql_select_linkproperty_04(self):
+        with self.assertRaisesRegex(
+            edgedb.InvalidReferenceError,
+            "link property 'since' should be accessed "
+            "from link, not from 'test::User'",
+        ):
+            await self.con.execute(
+                r'''
+                WITH MODULE test
+                SELECT
+                    Issue { since := (SELECT .owner)@since }
+                ''',
+            )
+
+    async def test_edgeql_select_linkproperty_05(self):
+        with self.assertRaisesRegex(
+            edgedb.InvalidReferenceError,
+            "link property 'since' should be accessed "
+            "from link, not from 'array<test::User>'",
+        ):
+            await self.con.execute(
+                r'''
+                WITH MODULE test
+                SELECT
+                    Issue { since := [.owner]@since }
+                ''',
+            )
+
     async def test_edgeql_select_if_else_01(self):
         await self.assert_query_result(
             r"""


Code review comments:
diff_hunk: @@ -264,6 +264,15 @@ def compile_path(expr: qlast.Path, *, ctx: context.ContextLevel) -> irast.Set:
             if ptr_expr.type == 'property':
                 # Link property reference; the source is the
                 # link immediately preceding this step in the path.
+                if path_tip.rptr is None:
+                    source = get_set_type(path_tip, ctx=ctx)
+                    s_dn = source.get_displayname(ctx.env.schema)
+                    raise errors.InvalidReferenceError(
path: edb/edgeql/compiler/setgen.py
text: @user1:
I'd use "unexpected reference to link property {ptr_name!r} outside of a path expression", and I think it should be an `EdgeQLSyntaxError` (we normally use `InvalidReferenceError` to indicate a syntactically-correct reference to a non-existent object).

@author:
I've changed error and message.

> we normally use `InvalidReferenceError` to indicate a syntactically-correct reference to a non-existent object

Thanks for the explanation. I raised an `InvalidReferenceError` following the message in the issue, but the syntax error here really looks more appropriate, given what is written in the issue:
> The only valid form of link property reference is .link@user2.