Add a compiler pass to analyze non-existing imported symbols #6726
Conversation
This PR does not add additional resolving functionality - no ResolvedMethod and ResolvedConstructor should be part of BindingMap.resolvedImport targets.
Akirathan
requested review from
4e6,
JaroslavTulach,
radeusgd and
hubertp
as code owners
Akirathan
added
the
CI: No changelog needed
| * Checks whether the exported symbols and defined entities metadata of the modules | ||
| * are correct. | ||
| */ | ||
| class ImportExportTest |
There was a problem hiding this comment.
I'd prefer the new tests to be written in Java than Scala, but be it...
There was a problem hiding this comment.
The reason why I chose Scala instead of Java for this particular test is that in this test, we traverse the IR and its metadata and that is way easier to do in Scala - since all the associated definitions are in Scala. I find IR manipulation in Java very clumsy, not even mentioning that IntelliJ is incapable of proper symbol resolution in such cases.
There was a problem hiding this comment.
A screenshot of IJ of ErrorCompilerTest.java that directly manipulates with IR:
There was a problem hiding this comment.
I am always pleased seeing other IDEs having problems understanding Java. I like to point out that NetBeans is WYSIWYG - it understands the source code exactly the same way as javac does unlike the other IDEs.
There was a problem hiding this comment.
Regardless of my silly jokes about IDEs, I believe writing such an amount of new Scala code isn't justified.
The test isn't using anything from existing testing infrastructure - e.g. it is not modification of existing Scala code. In such case the general direction is to: move away from Scala.
There was a problem hiding this comment.
I understand the general policy of moving away from Scala. However, code that accesses IR and checks its metadata is a major pain in the ass to write in Java. Accessing anything from IR.scala in Java is very difficult, and takes longer time to write. Even if we manage somehow to set up IntelliJ to provide proper symbol completion for Scala case objects and other nice Scala stuff from Java, how is one supposed to know that to access a case object one has to access .$MODULE$ field of some IR$Scope$Module$Import$ImportExportResaon$ class?
I am 100% in agreement with moving away from Scala one time. But IMHO as long as we don't migrate IR.scala to Java, it is not worth writing tests that traverse IR in Java. Any other code that does not access IR and its metadata, and just evaluates a module and checks its output (like PrintTest), I will happily code in Java.
| @@ -621,8 +621,8 @@ def commit_to_str(commit: Commit) -> str: | |||
|
|
|||
|
|
|||
| if __name__ == '__main__': | |||
| default_since = datetime.now() - timedelta(days=14) | |||
| default_until = datetime.now() | |||
| default_since: date = (datetime.now() - timedelta(days=14)).date() | |||
There was a problem hiding this comment.
That is a small fix to make this script usable in my systemd daily job.
| } else { | ||
| List(imp) | ||
| } | ||
| case None => List(imp) |
There was a problem hiding this comment.
So if we don't find the import we return.. the import? Shouldn't that return an error?
There was a problem hiding this comment.
I am not sure, to be honest. Usually, bindingMap.resolvedImports.find(_.importDef == imp) finds something. The only case when this will return None, is when ImportResolver was not run previously, which might happen, e.g., in a testing environment. What do you think? Should that return an error?
Previously, I had there bindingMap.resolvedImports.find(_.importDef == imp).getOrElse(throw new CompilerError(...), but that failed in the testing environment.
Akirathan
added
the
CI: Keep up to date
Akirathan
removed
the
CI: Keep up to date
|
There is a huge amount of Seems like transient issue. Restarting the jobs.
|
* develop: Link to new 101 tutorial and not deprecated one. (#6793) Add logs section to the bug template (#6798) Fix #6521: Main module function calls shouldn't use project namespace (#6719) Add project creation time to project metadata (#6780) Add a compiler pass to analyze non-existing imported symbols (#6726)
Fixes #5936
Pull Request Description
Add diagnosis for unresolved symbols in
from ... import sym1, sym2, ...statements.Important Notes
ImportSymbolAnalysis, that checks these statements and iterates through the symbols and checks if all the symbols can be resolved.BindingsMapmetadata.ImportExportTestthat creates various modules with various imports/exports and checks their generatedBindingMap.Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
./run ide build.