Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix nullary with side effects error #1437

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

olabusayoT
Copy link
Contributor

  • add () to test and trace definition/calls
  • add () to test defs

DAFFODIL-2152

- add () to test and trace definition/calls
- add () to test defs

DAFFODIL-2152
Copy link
Contributor

@jadams-tresys jadams-tresys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 with the change of collection.Seq to just Seq, assuming that is still valid.

@@ -22,7 +22,7 @@ import org.junit.Assert.fail
import org.junit.Test

class Thing() extends Serializable {
var things: Seq[Thing] = _
var things: collection.Seq[Thing] = _
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More collection.Seq instead of just Seq; is this necessary? I don't think it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're right. In this case, I can leave it as Seq, but I need to change the assert to not look for Collection.Seq, because in 2.13, the value will be Immutable.Seq.

Copy link
Contributor

@mbeckerle mbeckerle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1
It's a shame if scala forces us to use test() everywhere with the ().

I hate that, so I asked chatGPT if this could be avoided. Here's the output:

// Define an object to hold our "test" verb
object TestDSL {
  def test(): Unit = println("Running test...")
}

// Import implicit conversions to enable automatic resolution
import scala.language.implicitConversions

/**
 * Implicit conversion to enable `test` to be called without parentheses.
 *
 * ## Why This Works
 * 1. **Scala requires `()` for top-level methods returning `Unit`.**
 *    - If you define `def test: Unit = ...` at the top level, the compiler warns you.
 *    - The warning ensures that side-effecting methods use `()`, making their invocation explicit.
 *
 * 2. **However, Scala allows omitting `()` for methods on an instance or object.**
 *    - Example:
 *      ```scala
 *      object Example {
 *        def foo(): Unit = println("Hello")
 *      }
 *      Example.foo  // ✅ Allowed, no warning
 *      ```
 *    - Since `test` lives inside `TestDSL`, we can **implicitly resolve it through an instance**.
 *
 * 3. **Implicit Conversion Steps**
 *    - When `test` is called in `def myFunction = test`, the compiler does not find `test` locally.
 *    - It searches for **implicit conversions** and finds `liftTestDSL`.
 *    - `liftTestDSL` provides `TestDSL`, which contains `test()`.
 *    - Now `test` is resolved as **a method on an object** (`TestDSL.test`).
 *    - Since Scala allows **omitting `()` when calling methods on an object**, `test` works **without parentheses**.
 *
 * 4. **Why `DummyImplicit`?**
 *    - `DummyImplicit` is a special Scala type that ensures this implicit conversion does not interfere with others.
 *    - This avoids conflicts if there are multiple implicit conversions in scope.
 */
implicit def liftTestDSL(dummy: DummyImplicit): TestDSL.type = TestDSL

// Example function using `test` without parentheses
def myFunction = test  // Resolves to TestDSL.test()

@@ -132,50 +132,50 @@ object IBMTestsDPAEXT2 extends TdmlSuite {
class IBMTestsDPAEXT2 extends TdmlTests {
val tdmlSuite = IBMTestsDPAEXT2

@Test def simple_type_properties_text_boolean_13_01 = test
@Test def simple_type_properties_text_boolean_13_02 = test
@Test def simple_type_properties_text_boolean_13_01() = test()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't we just change the definition of test so that we don't have (and cannot) call it with the ()?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants