Fix #7986: Warn on cyclic references when looking for extension methods

[odersky]

Propagate cyclic reference errors when searching extension methods

[odersky]

The first attempt to simply propagate cyclic references breaks existing tests. We now issue a warning instead. On i7986.scala you get:

-- Warning: i7986.scala:3:46 ---------------------------------------------------
3 |def (project: Project) dependencies = project.name.dependencies // error, following a cyclic reference warning
  |                                      ^^^^^^^^^^^^
  |          An extension method was tried but could not be fully constructed
  |          since there was a cyclic reference involving method dependencies
-- [E008] Member Not Found Error: i7986.scala:3:51 -----------------------------
3 |def (project: Project) dependencies = project.name.dependencies // error, following a cyclic reference warning
  |                                      ^^^^^^^^^^^^^^^^^^^^^^^^^
  |                              value dependencies is not a member of String
1 warning found
1 error found

Unfortunately I cannot test that a warning is issued with a .check file since the .check files did not show the warning (maybe it was suppressed since there was an error?).

[anatoliykmetyuk]

Looks good as a temporary measure before we have a more systematic way to treat extension methods on type checking. Will direct people in the right direction.

[anatoliykmetyuk]

As a user, I do not necessarily understand what's a cyclic reference and how to fix it. Would it be exhaustive to say here that the user must explicitly specify the return types?

[odersky]

It turns out to be quite hard to figure out accurately that a return type is missing.

[anatoliykmetyuk]

The check files log only errors, the assumption being that warnings are irrelevant for negative tests since they do not cause the compilation failure. We could move the test to tests/neg-custom-args/fatal-warnings, then the warning will turn into an error and will be testable via a check file.

[anatoliykmetyuk]

You could probably do:

              case ex: CyclicReference =>
                if ex.denot.is(Extension) && ex.denot.name == name
                  throw ex
                else
                  errorTree(tree, ex, tree.sourcePos)

It fixes the failing tests.

[odersky]

There was something really funky going on. Basically, we had in dokka/BasicSupport:

extension (sym: Symbol)
  def documentation = sym.documentation match ...

How is this not an infinite loop? Well, name resolution tried the current method,
failed because of a cyclic reference and then fell back to an alternative
extension method elsewhere. So compilation relied crucially on a missing result type
in documentation. This is terrible! This PR plugs the specific hole, but
I fear there are others that could be exploited in user code.

Morale: Leaving out result types has an upside in that it detects compiler
bugs. But for robust software, it's better do add return types to all public methods.

[odersky]

@romanowski Can you check that my changes to BasicSupport are correct?

[romanowski]

Changes in scal3doc looks ok. I've take a look why we had documentation calling documentation and it seems that originally we used comment method that gets renamed later on and we simply changed it and since it worked we leaved it be wihtout realizing that we exploited the compiler.

[odersky]

#10902 provides a better fix