util-app: make the App#flags field and Flags class final

Problem

`App` is a trait that exposes non-final fields as `val`, which can result
in access ordering issues when fields are overridden and then accessed
by the base trait. While attempting to solve this problem, `Flags`
overrides introduce other complications, as there are multiple ways to
hook into flag parsing (via both `App#parseArgs` and `Flags#parseArgs`.

Solution

We move our `val` fields to `def` and expose the necessary properties that
can be overridden to achieve the existing behavior, just more consistently.
We make the `App#flags` def final and `Flags` class becomes final.
The `App#name` val is similarly made a `def`.

JIRA Issues: CSL-11231

Differential Revision: https://phabricator.twitter.biz/D723956

Author: enbnt

CHANGELOG.rst

@@ -29,6 +29,18 @@ Runtime Behavior Changes
 
 * util: Bump version of Jackson to 2.11.4. ``PHAB_ID=D727879``
 
+Breaking API Changes
+~~~~~~~~~~~~~~~~~~~~
+
+* util-app: the `c.t.app.App#flags` field is now a `final def` instead of a `val` to address
+  `override val` scenarios where the `c.t.app.App#flags` are accessed as part of construction,
+  resulting in a NullPointerException due to access ordering issues.
+  The `c.t.app.Flags` class is now made final. The `c.t.app.App#name` field has changed from
+  a `val` and is now a `def`. A new `c.t.app.App#includeGlobalFlags` def has been exposed, which
+  defaults to `true`. The `c.t.app#includeGlobalFlags` def can be overridden to `false`
+  (ex: `override protected def includeGlobalFlags: Boolean = false`) in order to skip discovery
+  of `GlobalFlag`s during flag parsing. ```PHAB_ID=D723956``
+
 21.8.0 (No 21.7.0 Release)
 --------------------------
 

util-app/src/main/scala/com/twitter/app/App.scala

@@ -42,12 +42,15 @@ import scala.util.control.NonFatal
 trait App extends ClosableOnce with CloseOnceAwaitably with Lifecycle {
 
   /** The name of the application, based on the classname */
-  val name: String = getClass.getName.stripSuffix("$")
+  def name: String = getClass.getName.stripSuffix("$")
 
-  /** The [[com.twitter.app.Flags]] instance associated with this application */
   //failfastOnFlagsNotParsed is called in the ctor of App.scala here which is a bad idea
   //as things like this can happen https://stackoverflow.com/questions/18138397/calling-method-from-constructor
-  val flag: Flags = new Flags(name, includeGlobal = true, failfastOnFlagsNotParsed)
+  private[this] val _flag: Flags =
+    new Flags(name, includeGlobal = includeGlobalFlags, failfastOnFlagsNotParsed)
+
+  /** The [[com.twitter.app.Flags]] instance associated with this application */
+  final def flag: Flags = _flag
 
   private var _args = Array[String]()
 
@@ -57,6 +60,9 @@ trait App extends ClosableOnce with CloseOnceAwaitably with Lifecycle {
   /** Whether or not to accept undefined flags */
   protected def allowUndefinedFlags: Boolean = false
 
+  /** Whether or not to include [[GlobalFlag GlobalFlag's]] when [[Flags]] are parsed */
+  protected def includeGlobalFlags: Boolean = true
+
   /**
    * Users of this code should override this to `true` so that
    * you fail-fast instead of being surprised at runtime by code that

util-app/src/main/scala/com/twitter/app/Flags.scala

@@ -81,7 +81,7 @@ object Flags {
  * be included during flag parsing. If false, only flags defined in the
  * application itself will be consulted.
  */
-class Flags(argv0: String, includeGlobal: Boolean, failFastUntilParsed: Boolean) {
+final class Flags(argv0: String, includeGlobal: Boolean, failFastUntilParsed: Boolean) {
   import com.twitter.app.Flags._
 
   def this(argv0: String, includeGlobal: Boolean) = this(argv0, includeGlobal, false)