Feedback sought: Optional Braces

[odersky]

I experimented with tab sizes a bit. I took a file I had written that used exclusively indentation syntax and
rendered it with tab sizes of 2, 3, and 4. Please bear with me since the file is about 300 lines long, but that way I hope to get a more representative impression.

Nullables.scala, tab size = 2

/** Operations for implementing a flow analysis for nullability */
object Nullables:
  import ast.tpd._

  /** A set of val or var references that are known to be not null, plus a set of
  *  variable references that are not known (anymore) to be not null
  */
  case class NotNullInfo(asserted: Set[TermRef], retracted: Set[TermRef]):
    assert((asserted & retracted).isEmpty)

    def isEmpty = this eq NotNullInfo.empty

    def retractedInfo = NotNullInfo(Set(), retracted)

    /** The sequential combination with another not-null info */
    def seq(that: NotNullInfo): NotNullInfo =
      if this.isEmpty then that
      else if that.isEmpty then this
      else NotNullInfo(
        this.asserted.union(that.asserted).diff(that.retracted),
        this.retracted.union(that.retracted).diff(that.asserted))

    /** The alternative path combination with another not-null info. Used to merge
    *  the nullability info of the two branches of an if.
    */
    def alt(that: NotNullInfo): NotNullInfo =
      NotNullInfo(this.asserted.intersect(that.asserted), this.retracted.union(that.retracted))

  object NotNullInfo:
    val empty = new NotNullInfo(Set(), Set())
    def apply(asserted: Set[TermRef], retracted: Set[TermRef]): NotNullInfo =
      if asserted.isEmpty && retracted.isEmpty then empty
      else new NotNullInfo(asserted, retracted)
  end NotNullInfo

  /** A pair of not-null sets, depending on whether a condition is `true` or `false` */
  case class NotNullConditional(ifTrue: Set[TermRef], ifFalse: Set[TermRef]):
    def isEmpty = this eq NotNullConditional.empty

  object NotNullConditional:
    val empty = new NotNullConditional(Set(), Set())
    def apply(ifTrue: Set[TermRef], ifFalse: Set[TermRef]): NotNullConditional =
      if ifTrue.isEmpty && ifFalse.isEmpty then empty
      else new NotNullConditional(ifTrue, ifFalse)
  end NotNullConditional

  /** An attachment that represents conditional flow facts established
  *  by this tree, which represents a condition.
  */
  private[typer] val NNConditional = Property.StickyKey[NotNullConditional]

  /** An attachment that represents unconditional flow facts established
    *  by this tree.
    */
  private[typer] val NNInfo = Property.StickyKey[NotNullInfo]

  /** An extractor for null comparisons */
  object CompareNull:

    /** Matches one of
    *
    *    tree == null, tree eq null, null == tree, null eq tree
    *    tree != null, tree ne null, null != tree, null ne tree
    *
    *  The second boolean result is true for equality tests, false for inequality tests
    */
    def unapply(tree: Tree)(using Context): Option[(Tree, Boolean)] = tree match
      case Apply(Select(l, _), Literal(Constant(null)) :: Nil) =>
        testSym(tree.symbol, l)
      case Apply(Select(Literal(Constant(null)), _), r :: Nil) =>
        testSym(tree.symbol, r)
      case _ =>
        None

    private def testSym(sym: Symbol, operand: Tree)(using Context) =
      if sym == defn.Any_== || sym == defn.Object_eq then Some((operand, true))
      else if sym == defn.Any_!= || sym == defn.Object_ne then Some((operand, false))
      else None

  end CompareNull

  /** An extractor for null-trackable references */
  object TrackedRef:
    def unapply(tree: Tree)(using Context): Option[TermRef] = tree.typeOpt match
      case ref: TermRef if isTracked(ref) => Some(ref)
      case _ => None
  end TrackedRef

  def isTracked(ref: TermRef)(using Context) =
    ref.isStable
    || { val sym = ref.symbol
        !ref.usedOutOfOrder
        && sym.span.exists
        && ctx.compilationUnit != null // could be null under -Ytest-pickler
        && ctx.compilationUnit.assignmentSpans.contains(sym.span.start)
      }

  /** The nullability context to be used after a case that matches pattern `pat`.
  *  If `pat` is `null`, this will assert that the selector `sel` is not null afterwards.
  */
  def afterPatternContext(sel: Tree, pat: Tree)(using Context) = (sel, pat) match
    case (TrackedRef(ref), Literal(Constant(null))) => ctx.addNotNullRefs(Set(ref))
    case _ => ctx

  /** The nullability context to be used for the guard and rhs of a case with
  *  given pattern `pat`. If the pattern can only match non-null values, this
  *  will assert that the selector `sel` is not null in these regions.
  */
  def caseContext(sel: Tree, pat: Tree)(using Context): Context = sel match
    case TrackedRef(ref) if matchesNotNull(pat) => ctx.addNotNullRefs(Set(ref))
    case _ => ctx

  private def matchesNotNull(pat: Tree)(using Context): Boolean = pat match
    case _: Typed | _: UnApply => true
    case Alternative(pats) => pats.forall(matchesNotNull)
    // TODO: Add constant pattern if the constant type is not nullable
    case _ => false

  extension (infos: List[NotNullInfo])

    /** Do the current not-null infos imply that `ref` is not null?
    *  Not-null infos are as a history where earlier assertions and retractions replace
    *  later ones (i.e. it records the assignment history in reverse, with most recent first)
    */
    @tailrec def impliesNotNull(ref: TermRef): Boolean = infos match
      case info :: infos1 =>
        if info.asserted.contains(ref) then true
        else if info.retracted.contains(ref) then false
        else infos1.impliesNotNull(ref)
      case _ =>
        false

    /** Add `info` as the most recent entry to the list of null infos. Assertions
    *  or retractions in `info` supersede infos in existing entries of `infos`.
    */
    def extendWith(info: NotNullInfo) =
      if info.isEmpty
        || info.asserted.forall(infos.impliesNotNull(_))
            && !info.retracted.exists(infos.impliesNotNull(_))
      then infos
      else info :: infos

    /** Retract all references to mutable variables */
    def retractMutables(using Context) =
      val mutables = infos.foldLeft(Set[TermRef]())((ms, info) =>
        ms.union(info.asserted.filter(_.symbol.is(Mutable))))
      infos.extendWith(NotNullInfo(Set(), mutables))

  end extension

  extension (ref: TermRef)
    def usedOutOfOrder(using Context): Boolean =
      val refSym = ref.symbol
      val refOwner = refSym.owner

      @tailrec def recur(s: Symbol): Boolean =
        s != NoSymbol
        && s != refOwner
        && (s.isOneOf(Lazy | Method) // not at the rhs of lazy ValDef or in a method (or lambda)
            || s.isClass // not in a class
              // TODO: need to check by-name parameter
            || recur(s.owner))

      refSym.is(Mutable) // if it is immutable, we don't need to check the rest conditions
      && refOwner.isTerm
      && recur(ctx.owner)
  end extension

  extension (tree: Tree)

    /* The `tree` with added nullability attachment */
    def withNotNullInfo(info: NotNullInfo): tree.type =
      if !info.isEmpty then tree.putAttachment(NNInfo, info)
      tree

    /* The nullability info of `tree` */
    def notNullInfo(using Context): NotNullInfo =
      stripInlined(tree).getAttachment(NNInfo) match
        case Some(info) if !ctx.erasedTypes => info
        case _ => NotNullInfo.empty

    /* The nullability info of `tree`, assuming it is a condition that evaluates to `c` */
    def notNullInfoIf(c: Boolean)(using Context): NotNullInfo =
      val cond = tree.notNullConditional
      if cond.isEmpty then tree.notNullInfo
      else tree.notNullInfo.seq(NotNullInfo(if c then cond.ifTrue else cond.ifFalse, Set()))

    /** The paths that are known to be not null if the condition represented
    *  by `tree` yields `true` or `false`. Two empty sets if `tree` is not
    *  a condition.
    */
    def notNullConditional(using Context): NotNullConditional =
      stripBlock(tree).getAttachment(NNConditional) match
        case Some(cond) if !ctx.erasedTypes => cond
        case _ => NotNullConditional.empty

    /** The current context augmented with nullability information of `tree` */
    def nullableContext(using Context): Context =
      val info = tree.notNullInfo
      if info.isEmpty then ctx else ctx.addNotNullInfo(info)

    /** The current context augmented with nullability information,
    *  assuming the result of the condition represented by `tree` is the same as
    *  the value of `c`.
    */
    def nullableContextIf(c: Boolean)(using Context): Context =
      val info = tree.notNullInfoIf(c)
      if info.isEmpty then ctx else ctx.addNotNullInfo(info)

    /** The context to use for the arguments of the function represented by `tree`.
    *  This is the current context, augmented with nullability information
    *  of the left argument, if the application is a boolean `&&` or `||`.
    */
    def nullableInArgContext(using Context): Context = tree match
      case Select(x, _) if !ctx.erasedTypes =>
        if tree.symbol == defn.Boolean_&& then x.nullableContextIf(true)
        else if tree.symbol == defn.Boolean_|| then x.nullableContextIf(false)
        else ctx
      case _ => ctx
  end extension
end Nullables

Nullables.scala, tab size = 3

/** Operations for implementing a flow analysis for nullability */
object Nullables:
   import ast.tpd._

   /** A set of val or var references that are known to be not null, plus a set of
   *   variable references that are not known (anymore) to be not null
   */
   case class NotNullInfo(asserted: Set[TermRef], retracted: Set[TermRef]):
      assert((asserted & retracted).isEmpty)

      def isEmpty = this eq NotNullInfo.empty

      def retractedInfo = NotNullInfo(Set(), retracted)

      /** The sequential combination with another not-null info */
      def seq(that: NotNullInfo): NotNullInfo =
         if this.isEmpty then that
         else if that.isEmpty then this
         else NotNullInfo(
            this.asserted.union(that.asserted).diff(that.retracted),
            this.retracted.union(that.retracted).diff(that.asserted))

      /** The alternative path combination with another not-null info. Used to merge
      *   the nullability info of the two branches of an if.
      */
      def alt(that: NotNullInfo): NotNullInfo =
         NotNullInfo(this.asserted.intersect(that.asserted), this.retracted.union(that.retracted))

   object NotNullInfo:
      val empty = new NotNullInfo(Set(), Set())
      def apply(asserted: Set[TermRef], retracted: Set[TermRef]): NotNullInfo =
         if asserted.isEmpty && retracted.isEmpty then empty
         else new NotNullInfo(asserted, retracted)
   end NotNullInfo

   /** A pair of not-null sets, depending on whether a condition is `true` or `false` */
   case class NotNullConditional(ifTrue: Set[TermRef], ifFalse: Set[TermRef]):
      def isEmpty = this eq NotNullConditional.empty

   object NotNullConditional:
      val empty = new NotNullConditional(Set(), Set())
      def apply(ifTrue: Set[TermRef], ifFalse: Set[TermRef]): NotNullConditional =
         if ifTrue.isEmpty && ifFalse.isEmpty then empty
         else new NotNullConditional(ifTrue, ifFalse)
   end NotNullConditional

   /** An attachment that represents conditional flow facts established
   *   by this tree, which represents a condition.
   */
   private[typer] val NNConditional = Property.StickyKey[NotNullConditional]

   /** An attachment that represents unconditional flow facts established
      *   by this tree.
      */
   private[typer] val NNInfo = Property.StickyKey[NotNullInfo]

   /** An extractor for null comparisons */
   object CompareNull:

      /** Matches one of
      *
      *      tree == null, tree eq null, null == tree, null eq tree
      *      tree != null, tree ne null, null != tree, null ne tree
      *
      *   The second boolean result is true for equality tests, false for inequality tests
      */
      def unapply(tree: Tree)(using Context): Option[(Tree, Boolean)] =
         tree match
         case Apply(Select(l, _), Literal(Constant(null)) :: Nil) =>
            testSym(tree.symbol, l)
         case Apply(Select(Literal(Constant(null)), _), r :: Nil) =>
            testSym(tree.symbol, r)
         case _ =>
            None

      private def testSym(sym: Symbol, operand: Tree)(using Context) =
         if sym == defn.Any_== || sym == defn.Object_eq then Some((operand, true))
         else if sym == defn.Any_!= || sym == defn.Object_ne then Some((operand, false))
         else None

   end CompareNull

   /** An extractor for null-trackable references */
   object TrackedRef:
      def unapply(tree: Tree)(using Context): Option[TermRef] =
         tree.typeOpt match
         case ref: TermRef if isTracked(ref) => Some(ref)
         case _ => None
   end TrackedRef

   def isTracked(ref: TermRef)(using Context) =
      ref.isStable
      || {
         val sym = ref.symbol
         !ref.usedOutOfOrder
         && sym.span.exists
         && ctx.compilationUnit != null // could be null under -Ytest-pickler
         && ctx.compilationUnit.assignmentSpans.contains(sym.span.start)
      }

   /** The nullability context to be used after a case that matches pattern `pat`.
   *   If `pat` is `null`, this will assert that the selector `sel` is not null afterwards.
   */
   def afterPatternContext(sel: Tree, pat: Tree)(using Context) =
      (sel, pat) match
      case (TrackedRef(ref), Literal(Constant(null))) => ctx.addNotNullRefs(Set(ref))
      case _ => ctx

   /** The nullability context to be used for the guard and rhs of a case with
   *   given pattern `pat`. If the pattern can only match non-null values, this
   *   will assert that the selector `sel` is not null in these regions.
   */
   def caseContext(sel: Tree, pat: Tree)(using Context): Context =
      sel match
      case TrackedRef(ref) if matchesNotNull(pat) => ctx.addNotNullRefs(Set(ref))
      case _ => ctx

   private def matchesNotNull(pat: Tree)(using Context): Boolean =
      pat match
      case _: Typed | _: UnApply => true
      case Alternative(pats) => pats.forall(matchesNotNull)
      // TODO: Add constant pattern if the constant type is not nullable
      case _ => false

   extension (infos: List[NotNullInfo])

      /** Do the current not-null infos imply that `ref` is not null?
      *   Not-null infos are as a history where earlier assertions and retractions replace
      *   later ones (i.e. it records the assignment history in reverse, with most recent first)
      */
      @tailrec def impliesNotNull(ref: TermRef): Boolean =
         infos match
         case info :: infos1 =>
            if info.asserted.contains(ref) then true
            else if info.retracted.contains(ref) then false
            else infos1.impliesNotNull(ref)
         case _ =>
            false

      /** Add `info` as the most recent entry to the list of null infos. Assertions
      *   or retractions in `info` supersede infos in existing entries of `infos`.
      */
      def extendWith(info: NotNullInfo) =
         if info.isEmpty
            || info.asserted.forall(infos.impliesNotNull(_))
               && !info.retracted.exists(infos.impliesNotNull(_))
         then infos
         else info :: infos

      /** Retract all references to mutable variables */
      def retractMutables(using Context) =
         val mutables = infos.foldLeft(Set[TermRef]())((ms, info) =>
            ms.union(info.asserted.filter(_.symbol.is(Mutable))))
         infos.extendWith(NotNullInfo(Set(), mutables))

   end extension

   extension (ref: TermRef)
      def usedOutOfOrder(using Context): Boolean =
         val refSym = ref.symbol
         val refOwner = refSym.owner

         @tailrec def recur(s: Symbol): Boolean =
            s != NoSymbol
            && s != refOwner
            && (s.isOneOf(Lazy | Method) // not at the rhs of lazy ValDef or in a method (or lambda)
               || s.isClass // not in a class
                     // TODO: need to check by-name parameter
               || recur(s.owner)
               )

         refSym.is(Mutable) // if it is immutable, we don't need to check the rest conditions
         && refOwner.isTerm
         && recur(ctx.owner)
   end extension

   extension (tree: Tree)

      /* The `tree` with added nullability attachment */
      def withNotNullInfo(info: NotNullInfo): tree.type =
         if !info.isEmpty then tree.putAttachment(NNInfo, info)
         tree

      /* The nullability info of `tree` */
      def notNullInfo(using Context): NotNullInfo =
         stripInlined(tree).getAttachment(NNInfo) match
         case Some(info) if !ctx.erasedTypes => info
         case _ => NotNullInfo.empty

      /* The nullability info of `tree`, assuming it is a condition that evaluates to `c` */
      def notNullInfoIf(c: Boolean)(using Context): NotNullInfo =
         val cond = tree.notNullConditional
         if cond.isEmpty then tree.notNullInfo
         else tree.notNullInfo.seq(NotNullInfo(if c then cond.ifTrue else cond.ifFalse, Set()))

      /** The paths that are known to be not null if the condition represented
      *   by `tree` yields `true` or `false`. Two empty sets if `tree` is not
      *   a condition.
      */
      def notNullConditional(using Context): NotNullConditional =
         stripBlock(tree).getAttachment(NNConditional) match
            case Some(cond) if !ctx.erasedTypes => cond
            case _ => NotNullConditional.empty

      /** The current context augmented with nullability information of `tree` */
      def nullableContext(using Context): Context =
         val info = tree.notNullInfo
         if info.isEmpty then ctx else ctx.addNotNullInfo(info)

      /** The current context augmented with nullability information,
      *   assuming the result of the condition represented by `tree` is the same as
      *   the value of `c`.
      */
      def nullableContextIf(c: Boolean)(using Context): Context =
         val info = tree.notNullInfoIf(c)
         if info.isEmpty then ctx else ctx.addNotNullInfo(info)

      /** The context to use for the arguments of the function represented by `tree`.
      *   This is the current context, augmented with nullability information
      *   of the left argument, if the application is a boolean `&&` or `||`.
      */
      def nullableInArgContext(using Context): Context = tree match
         case Select(x, _) if !ctx.erasedTypes =>
            if tree.symbol == defn.Boolean_&& then x.nullableContextIf(true)
            else if tree.symbol == defn.Boolean_|| then x.nullableContextIf(false)
            else ctx
         case _ => ctx
   end extension
end Nullables   

Nullables.scala, tab size = 4

/** Operations for implementing a flow analysis for nullability */
object Nullables:
    import ast.tpd._

    /** A set of val or var references that are known to be not null, plus a set of
    *  variable references that are not known (anymore) to be not null
    */
    case class NotNullInfo(asserted: Set[TermRef], retracted: Set[TermRef]):
        assert((asserted & retracted).isEmpty)

        def isEmpty = this eq NotNullInfo.empty

        def retractedInfo = NotNullInfo(Set(), retracted)

        /** The sequential combination with another not-null info */
        def seq(that: NotNullInfo): NotNullInfo =
            if this.isEmpty then that
            else if that.isEmpty then this
            else NotNullInfo(
                this.asserted.union(that.asserted).diff(that.retracted),
                this.retracted.union(that.retracted).diff(that.asserted))

        /** The alternative path combination with another not-null info. Used to merge
        *    the nullability info of the two branches of an if.
        */
        def alt(that: NotNullInfo): NotNullInfo =
            NotNullInfo(this.asserted.intersect(that.asserted), this.retracted.union(that.retracted))

    object NotNullInfo:
        val empty = new NotNullInfo(Set(), Set())
        def apply(asserted: Set[TermRef], retracted: Set[TermRef]): NotNullInfo =
            if asserted.isEmpty && retracted.isEmpty then empty
            else new NotNullInfo(asserted, retracted)
    end NotNullInfo

    /** A pair of not-null sets, depending on whether a condition is `true` or `false` */
    case class NotNullConditional(ifTrue: Set[TermRef], ifFalse: Set[TermRef]):
        def isEmpty = this eq NotNullConditional.empty

    object NotNullConditional:
        val empty = new NotNullConditional(Set(), Set())
        def apply(ifTrue: Set[TermRef], ifFalse: Set[TermRef]): NotNullConditional =
            if ifTrue.isEmpty && ifFalse.isEmpty then empty
            else new NotNullConditional(ifTrue, ifFalse)
    end NotNullConditional

    /** An attachment that represents conditional flow facts established
    *    by this tree, which represents a condition.
    */
    private[typer] val NNConditional = Property.StickyKey[NotNullConditional]

    /** An attachment that represents unconditional flow facts established
        *    by this tree.
        */
    private[typer] val NNInfo = Property.StickyKey[NotNullInfo]

    /** An extractor for null comparisons */
    object CompareNull:

        /** Matches one of
        *
        *        tree == null, tree eq null, null == tree, null eq tree
        *        tree != null, tree ne null, null != tree, null ne tree
        *
        *    The second boolean result is true for equality tests, false for inequality tests
        */
        def unapply(tree: Tree)(using Context): Option[(Tree, Boolean)] =
            tree match
            case Apply(Select(l, _), Literal(Constant(null)) :: Nil) =>
                testSym(tree.symbol, l)
            case Apply(Select(Literal(Constant(null)), _), r :: Nil) =>
                testSym(tree.symbol, r)
            case _ =>
                None

        private def testSym(sym: Symbol, operand: Tree)(using Context) =
            if sym == defn.Any_== || sym == defn.Object_eq then Some((operand, true))
            else if sym == defn.Any_!= || sym == defn.Object_ne then Some((operand, false))
            else None

    end CompareNull

    /** An extractor for null-trackable references */
    object TrackedRef:
        def unapply(tree: Tree)(using Context): Option[TermRef] =
            tree.typeOpt match
            case ref: TermRef if isTracked(ref) => Some(ref)
            case _ => None
    end TrackedRef

    def isTracked(ref: TermRef)(using Context) =
        ref.isStable
        || {
            val sym = ref.symbol
            !ref.usedOutOfOrder
            && sym.span.exists
            && ctx.compilationUnit != null // could be null under -Ytest-pickler
            && ctx.compilationUnit.assignmentSpans.contains(sym.span.start)
        }

    /** The nullability context to be used after a case that matches pattern `pat`.
    *    If `pat` is `null`, this will assert that the selector `sel` is not null afterwards.
    */
    def afterPatternContext(sel: Tree, pat: Tree)(using Context) =
        (sel, pat) match
        case (TrackedRef(ref), Literal(Constant(null))) => ctx.addNotNullRefs(Set(ref))
        case _ => ctx

    /** The nullability context to be used for the guard and rhs of a case with
    *    given pattern `pat`. If the pattern can only match non-null values, this
    *    will assert that the selector `sel` is not null in these regions.
    */
    def caseContext(sel: Tree, pat: Tree)(using Context): Context =
        sel match
        case TrackedRef(ref) if matchesNotNull(pat) => ctx.addNotNullRefs(Set(ref))
        case _ => ctx

    private def matchesNotNull(pat: Tree)(using Context): Boolean =
        pat match
        case _: Typed | _: UnApply => true
        case Alternative(pats) => pats.forall(matchesNotNull)
        // TODO: Add constant pattern if the constant type is not nullable
        case _ => false

    extension (infos: List[NotNullInfo])

        /** Do the current not-null infos imply that `ref` is not null?
        *    Not-null infos are as a history where earlier assertions and retractions replace
        *    later ones (i.e. it records the assignment history in reverse, with most recent first)
        */
        @tailrec def impliesNotNull(ref: TermRef): Boolean =
            infos match
            case info :: infos1 =>
                if info.asserted.contains(ref) then true
                else if info.retracted.contains(ref) then false
                else infos1.impliesNotNull(ref)
            case _ =>
                false

        /** Add `info` as the most recent entry to the list of null infos. Assertions
        *    or retractions in `info` supersede infos in existing entries of `infos`.
        */
        def extendWith(info: NotNullInfo) =
            if info.isEmpty
                || info.asserted.forall(infos.impliesNotNull(_))
                    && !info.retracted.exists(infos.impliesNotNull(_))
            then infos
            else info :: infos

        /** Retract all references to mutable variables */
        def retractMutables(using Context) =
            val mutables = infos.foldLeft(Set[TermRef]())((ms, info) =>
                ms.union(info.asserted.filter(_.symbol.is(Mutable))))
            infos.extendWith(NotNullInfo(Set(), mutables))

    end extension

    extension (ref: TermRef)
        def usedOutOfOrder(using Context): Boolean =
            val refSym = ref.symbol
            val refOwner = refSym.owner

            @tailrec def recur(s: Symbol): Boolean =
                s != NoSymbol
                && s != refOwner
                && (s.isOneOf(Lazy | Method) // not at the rhs of lazy ValDef or in a method (or lambda)
                    || s.isClass // not in a class
                            // TODO: need to check by-name parameter
                    || recur(s.owner)
                    )

            refSym.is(Mutable) // if it is immutable, we don't need to check the rest conditions
            && refOwner.isTerm
            && recur(ctx.owner)
    end extension

    extension (tree: Tree)

        /* The `tree` with added nullability attachment */
        def withNotNullInfo(info: NotNullInfo): tree.type =
            if !info.isEmpty then tree.putAttachment(NNInfo, info)
            tree

        /* The nullability info of `tree` */
        def notNullInfo(using Context): NotNullInfo =
            stripInlined(tree).getAttachment(NNInfo) match
            case Some(info) if !ctx.erasedTypes => info
            case _ => NotNullInfo.empty

        /* The nullability info of `tree`, assuming it is a condition that evaluates to `c` */
        def notNullInfoIf(c: Boolean)(using Context): NotNullInfo =
            val cond = tree.notNullConditional
            if cond.isEmpty then tree.notNullInfo
            else tree.notNullInfo.seq(NotNullInfo(if c then cond.ifTrue else cond.ifFalse, Set()))

        /** The paths that are known to be not null if the condition represented
        *    by `tree` yields `true` or `false`. Two empty sets if `tree` is not
        *    a condition.
        */
        def notNullConditional(using Context): NotNullConditional =
            stripBlock(tree).getAttachment(NNConditional) match
                case Some(cond) if !ctx.erasedTypes => cond
                case _ => NotNullConditional.empty

        /** The current context augmented with nullability information of `tree` */
        def nullableContext(using Context): Context =
            val info = tree.notNullInfo
            if info.isEmpty then ctx else ctx.addNotNullInfo(info)

        /** The current context augmented with nullability information,
        *    assuming the result of the condition represented by `tree` is the same as
        *    the value of `c`.
        */
        def nullableContextIf(c: Boolean)(using Context): Context =
            val info = tree.notNullInfoIf(c)
            if info.isEmpty then ctx else ctx.addNotNullInfo(info)

        /** The context to use for the arguments of the function represented by `tree`.
        *    This is the current context, augmented with nullability information
        *    of the left argument, if the application is a boolean `&&` or `||`.
        */
        def nullableInArgContext(using Context): Context = tree match
            case Select(x, _) if !ctx.erasedTypes =>
                if tree.symbol == defn.Boolean_&& then x.nullableContextIf(true)
                else if tree.symbol == defn.Boolean_|| then x.nullableContextIf(false)
                else ctx
            case _ => ctx
    end extension
end Nullables

To my eyes, there’s a clear winner: tabsize 3. tabsize 2 is a bit too small, and tabsize 4 feels clunky. But 3 is perfect. There are also some specific reasons why 3 is better than 4, namely:

• it does not align with def or val.
• it does align with if and && and ||.

You have to see for yourself why that makes a difference.

That only leaves the problem that tabsize 3 will be even harder to establish than 4 since it is so uncommon…

[bjornregnell]

I can live with 3 spaces, and agree with that it’s easier to read. It’s wider (bad) but easier to read (good).

I don’t mind it’s uncommonness - if its stated the preferred width I think it can be established.

But I would not myself want to use 4 spaces, even if it was the officially preferred width.

[bjornregnell]

@curoli

As I said: there is a valid concern that Scala will gradually turn into a language primarily used for teaching and research, and less for actual software engineering.

Yes, we want Scala to be both a widely taught language and a widely used language. Java’s success was to a large extent based on that it, for a long period, has dominated as teaching language at universities around the world, providing an increasing number of developers who know the language…

I believe it is indeed possible to balance the language design so that it fits both teaching and practice.

[bjornregnell]

It might be difficult to get those who stick to “old” braceful style to abandon 2 space indent, and I guess we will have braceful and braceless styles mixed, even in the same file, for a long time.

And perhaps editors will not very soon support “brace-sensitive indent size” where 3 spaces are inserted in braceless indent and 2 spaces are inserted in braceful indent in the same file…

Perhaps this will be the toughest challenge if trying to standardize on 3 spaces in braceless code? The mixed brace style code, I mean.

[bmeesters]

I agree. Even though I am (mostly) in favor of the braceless style, I am not sure yet whether it is gonna fly within the company where I work. The more changes there are, the less likely I expect it wil be accepted.

On a more personal note, I find 4 spaces way too much. Scala generally takes up more horizontal space than Python and if you use line length limiter you burn through this quite quick.

[mbloms]

Leaving aside for a moment which indentation width is best, I think the challenge of changing from e.g. 2 to 3 spaces is exaggerated.

I might be wrong, but I think what will probably happen is that people will start writing new code, and in particular new source files without braces, rather than going around and changing old code. Having editors indent 3 spaces by default in new source files wouldn’t be too hard. For existing code, I think most IDE’s infer the indentation based on what’s in the current file.

Everyone won’t follow that standard anyway. When I was new to Scala, I always indented 4 spaces, because that’s what I did in all other languages, and I mostly used sublime, which indented 4 spaces by default. There are other examples:

@eyalroth

Speaking of indentation, scalafmt and codebase migrations, it’s just so happen that at my current place of employment, we wish to move to scalafamt. Alas, most of our code has a tab size of 4 spaces

Changing the standard would be a incremental process, and not everyone would follow it. That doesn’t really make it undoable.

[bjornregnell]

Yes, perhaps you’re right that it might work to change from 2 to 3 indent spaces gradually and eventually with focus on new code.

Perhaps an official Scala 3 poll on 2|3|4 indent in braceless code sent out to the community based on Martins nice example code above?

[Sciss]

I agree - indent size 3 is optimal; 2 is “ok” but sometimes a strain on the eyes without braces, 4 is too wide.

Anecdotally, when I moved from Java to Scala, I started out with 3 spaces because going from 4 to 2 was too hard for me to read. Two years or so later, I started reformatting all my code from 3 spaces to 2 spaces instead.

[esarbe]

During my – admittedly limited – experiments with Dotty I’ve come to very much appreciate the quiet/indentation syntax.
One thing I’ve been having issues with with is the already mentioned inconsistencies with regards line endings / new blocks. (This is of course not an issue for the braces-syntax.)
I find myself again and again having to thing about what construct requires what line ending. I would favor a more regular solution even at the cost of added verbosity.

[morgen-peschke]

@Jasper-M

I guess it should be possible to define <| or whatever else a DSL author chooses as a simple macro that optimizes the unnecessary function values away.

It seems like a big step backwards to have to resort to a macro to get what’s currently pretty basic functionality

[Jasper-M]

I don’t agree. IMO the basic functionality should work like this:

task(
  val x = ...
  val y = ...
  f(x, y)
)

That indented code after an operator is parsed as a block is a bonus.

Plus historically it has often been the case that user-code solutions were preferred over adding special constructs to the language (such as in this case passing arguments between braces instead of parentheses).

[morgen-peschke]

@Jasper-M

IMO the basic functionality should work like this:

I’m not talking about what I think it should look like, I’m talking about how it currently works. Right now, in Scala 2, the language makes it easier to to build a DSL, not harder. You don’t have to resort to a macro to create something that looks like it is part of the language, because braces are treated as a block.

Removing this feature adds a non-trivial barrier to writing that first embedded DSL, which would be an unfortunate step backwards.

[esarbe]

@odersky

Points in favor of with

• The language grammar becomes more regular, and will be easier to explain.
• The visual appearance also becomes more regular, at the price of more verbosity.

These for me are very strong arguments for with. Why wouldn’t you want a language to become more regular and easier to explain, especially with focus on teaching?

[implisci]

Inside braces, 2 spaces feels fine
Outside, 4 spaces is roomy and spacious
What to do with whom and where?
8 spaces says Go
Aaargh No!
Split the difference between 2 and 4 as 3
Or maybe 0 to 8, the spaces union
For a more perfect Scala fusion

[Jasper-M]

@morgen-peschke

You don’t have to resort to a macro to create something that looks like it is part of the language, because braces are treated as a block.

Actually I agree on this point but by now brace-optional syntax is a fact of life, so I guess we should make the best of it. And IMO all other suggestions to start a block are not good in general, and most would be terrible for DSLs.

This one might make the simpler DSLs a bit harder to implement (but still, you don’t need a macro) but at least it makes it possible to implement interesting DSLs.

[morgen-peschke]

@Jasper-M

This one might make the simpler DSLs a bit harder to implement (but still, you don’t need a macro) but at least it makes it possible to implement interesting DSLs.

Fair enough, just getting flashbacks to having to manually create Java Builders to work around the lack of named parameters.

I get that harm mitigation is a reasonable thing¹. I’m also concerned that the way it is currently manifesting is “how we can adjust the rest of the language to accommodate this,” rather than, “how can we mitigate risk by isolating this until we know more.”

I’m worried this whole mess is well on it’s way to snowballing from “hey, what do you think of this experiment,” to frantic patches touching every other aspect of the language, in ways that are going to make my life much less pleasant.

¹ From what I can tell, we’re very much in “lipstick on a pig” territory

[dlangdon]

Honestly none of you “weird” examples look bad to me. I don’t think I would find them confusing and/or unreadable. Sadly there is just too much subjectiveness in this topic. What “feels right” for some might be terrible for others and vice versa.

[dcsobral]

Mind you, the syntax used was this:

[details=(Minimized to make people hate me less)]
...
[/details]

Which is different than the gist you posted.

[lihaoyi]

So I just ran @bjornregnell’s survey in my company’s engineering team, with the same samples and prompts. These folks are basically the complete opposite of @bjornregnell’s students:

• Generally 5-10 years in industry, early- to mid-career folks
• Worked at multiple places in multiple languages before joining us
• Currently using Scala day-to-day at work, but not “enthusiasts” in the language itself.

Current set of responses is:

Prompt	Votes
Optional braces but add keyword with	1
Optional braces but add delimiter colon	17
Optional braces, nothing extra	4
“Why can’t they just stick to the old way”	19

(The last prompt was not asked, but came up organically and was voted up)

A lot of the concern around this effort is around the migration cost: migrating our backend systems (currently on 2.12), migrating Apache Spark (which we just got onto 2.12), which is what you would expect in an environment with a lot of stuff we maintain.

But assuming the last option isn’t on the table right now due to @odersky’s BDFL card, Optional braces but add delimited colon is currently the winner by an overwhelming margin.

[soronpo]

@lihaoyi

Current set of responses is:

Prompt	Votes
Optional braces but add keyword with	1
Optional braces but add delimiter colon	17
Optional braces, nothing extra	4
“Why can’t they just stick to the old way”	19

Was it possible to vote for more than one option?