Anvil fails silently when you set a property on a view that won't accept it

[lalunamel]

Hi!

I've spent a while debugging this issue and I think I found the root cause.

The problem

Setting a property on a view that doesn't accept that property fails silently.

Example code

drawerLayout {
  textView {
    text("content")
  }
  textView {
    text("drawer")
    layoutGravity(LEFT)
  }
}

the second child of drawerLayout will not have its layout_gravity set to LEFT because in BaseDSL.java in set,

case "layoutGravity":
                if (v.getLayoutParams() instanceof LinearLayout.LayoutParams && value instanceof Integer) {
                    ((LinearLayout.LayoutParams) v.getLayoutParams()).gravity = (int) value;
                    return true;
                } else if (v.getLayoutParams() instanceof FrameLayout.LayoutParams && value instanceof Integer) {
                    ((FrameLayout.LayoutParams) v.getLayoutParams()).gravity = (int) value;
                    return true;
                }
                break;

v.getLayoutParams() doesn't return layout params belonging to a linear layout nor a frame layout (they belong to a drawer layout).

The fix

If the program reaches one of these control statements but does not pass the tests laid out in the if statements, the user should be notified (by throwing an exception, logging a warning, or something)

Related but tangential grumblings

Relevant to Anvil

There are definitely more things than LinearLayout and FrameLayout that can have layout_gravity. Those things include TextView as eg. above and the root of my issue, DrawerLayout which if it doesn't have a second child with layout_gravity will not work properly.

Not relevant to Anvil but I'm frustrated and I'll say them anyway

Getting to the bottom of this issue has been frustrating and taken more time than I like, but I mostly blame the Android framework for pushing their constraints (ordering of child views, setting layout gravity) onto consumers of the framework. I should not have to set those things in order to make a drawer. See Leaky Abstraction.

[zserge]

Thanks for rising this topic! Starting with Anvil 0.5 there can be more than one DSL in the chain. Users may register their own attribute setters for certain attributes or views. So no particular attribute handler can be sure that it's the last one and that the attribute can't be set to the view.

If you want to be informed on an unsupported attribute - the correct place to handle it would be next to this line: https://github.com/zserge/anvil/blob/master/anvil/src/main/java/trikita/anvil/Anvil.java#L335

On the other hand, I'm not a big fan of printing uncontrolled logs from the libraries. Throwing exceptions would be probably even worse. So if you could think of a nice way to inform user that would not be too intrusive - feel free to make a pull request!

Answering your second question, the condition is not checking whether the view is a LinearLayout or a FrameLayout. It's checking whether the views is inside one of those. The core DSL knows nothing about DrawerLayout, since it's not part of the Android SDK. DrawerLayout.LayoutParams should be handled in this class - https://github.com/zserge/anvil/blob/master/anvil-support-v4/src/main/java/trikita/anvil/support/core/ui/SupportCoreUiDSL.java but they are not, because of how current code generator works. I'll see if I can find a way to modify the generator or to just add layout_gravity manually as a quirk. May I ask you to open a separate issue for that?

P.S. the current "silent" behavior is taken from how XMLs used to treat unsupported attributes in the days when Anvil was started - they silently ignored such attributes.