native icon indicating copy to clipboard operation
native copied to clipboard

New static Java Strings are impossible to use in switch statements

Open TheLastGimbus opened this issue 1 year ago • 3 comments
trafficstars

Hi!

Previously, i used jni.BluetoothDevice.ACTION_SOMETHING_SOMETHING static strings elegantly in switch statement, which made everything very elegant:

void onReceive(jni.Context context, jni.Intent intent) {
  switch (intent.getAction().toDString()) {
    case jni.BluetoothAdapter.ACTION_STATE_CHANGED:
      final btState = intent.getIntExtra(
          jni.BluetoothAdapter.EXTRA_STATE, -1);
      switch (btState) {
        case jni.BluetoothAdapter.STATE_ON:
          _isEnabledCtrl.add(true);
          break;
        case jni.BluetoothAdapter.STATE_OFF:
          _isEnabledCtrl.add(false);
          break;
      }
      break;
    case jni.BluetoothDevice.ACTION_BOND_STATE_CHANGED:
      final extraDev = _getExtraDev(intent);
      final bondState = intent.getIntExtra(
          jni.BluetoothDevice.EXTRA_BOND_STATE, -1);
      switch (bondState) {
        case jni.BluetoothDevice.BOND_BONDED:
          _pairedDevicesCtrl.add(
            _pairedDevicesCtrl.value
              ..add(
                _androidDevToDart(extraDev.dev),
              ),
          );
          break;

	...
	...

that's thanks to way they were simply statically generated:

static const ACTION_NAME_CHANGED =
      r"""android.bluetooth.device.action.NAME_CHANGED""";

now, they are accesed through _class.staticFieldId, which makes it not-const for Dart and thus excluding it from switch

/// from: static public final java.lang.String ACTION_PAIRING_REQUEST
/// The returned object must be released after use, by calling the [release] method.
static jni.JString get ACTION_PAIRING_REQUEST =>
    _id_ACTION_PAIRING_REQUEST.get(_class, const jni.JStringType());

static final _id_ACTION_UUID = _class.staticFieldId(
  r'ACTION_UUID',
  r'Ljava/lang/String;',
);

/// from: static public final java.lang.String ACTION_UUID
/// The returned object must be released after use, by calling the [release] method.
static jni.JString get ACTION_UUID =>
    _id_ACTION_UUID.get(_class, const jni.JStringType());
...

intrestingly enough, int statics are generated directly:

/// from: static public final int ADDRESS_TYPE_PUBLIC
static const ADDRESS_TYPE_PUBLIC = 0;

/// from: static public final int ADDRESS_TYPE_RANDOM
static const ADDRESS_TYPE_RANDOM = 1;
...

...which does make it switch-able


I know there are probably some complicated memory/security/magic reasons behind this, but at the end of the day... please, do I really have to if-elseif-elseif-elseif, or switch case _ when ... {} case _ when ... like a caveman :worried: ?

TheLastGimbus avatar Aug 11 '24 14:08 TheLastGimbus

I believe we did this due to troubles translating the encoding from Java Strings to Dart Strings:

  • https://github.com/dart-lang/native/issues/792

@HosseinYousefi @mahesh-hegde I don't believe we considered the use of switch statements and expressions. Should we revisit our decision? (Also inside a switch, we'd probably want to load the Java String into Dart once, and then do Dart string comparisons.)

dcharkes avatar Aug 12 '24 10:08 dcharkes

Looking briefly, seems like core problem is with escaping stuff... won't code units do the job?

void main() {
  final x = '\n';
  print(x.codeUnitAt(0));  // prints 10
  // 10 in hex -> 'A'
  print('new \u{000A} line');
}
10
new 
 line

TheLastGimbus avatar Aug 13 '24 10:08 TheLastGimbus

That's fixable using u{...}. The point is usually you don't actually want to serialize/deserialize strings. Integers don't have this problem.

In most cases we either want to pass a Java string to a method that accepts it or compare it with another Java string. Generating Dart strings requires an extra serialization/deserialization. Comparing the java strings using == (which works like Dart as it calls Java's equals) is faster although it might be a bit uglier.

HosseinYousefi avatar Aug 14 '24 16:08 HosseinYousefi

hm...

anyway, switch-case'ing some ACTION_SOMETHING_SOMETHING and EXTRA_SOMETHING from Intents and broadcasts is what i see myself doing 90% of time using Java api's, so this would be crazy useful :+1:

TheLastGimbus avatar Aug 22 '24 13:08 TheLastGimbus

@dcharkes @HosseinYousefi any new thoughts on this? I still need it very much :pray:

TheLastGimbus avatar Dec 19 '24 20:12 TheLastGimbus

@HosseinYousefi @mahesh-hegde I don't believe we considered the use of switch statements and expressions. Should we revisit our decision? (Also inside a switch, we'd probably want to load the Java String into Dart once, and then do Dart string comparisons.)

Sorry, missed this mention.

Yes initially we didn't consider this use case as far as I can remember.

If we can break the API, we can probably give a choice to the user, by generating static final strings as a compound dart type. (Like ENUM values have fields in some languages)

class JavaStringConstant {
  String stringValue;
  JString jValue;
}

Another option, which keeps API compatibility with previous generated code is to generate a suffixed value, like ACTIIN_SOMETHING$string_value (the choice of such convention to avoid edge case collisions is left to you) - looks a little ugly (but hey, some of us write shell scripts for a living).

get ACTION_SOMETHING {
  // return jni ref
}

const ACTION_SOMETHING$stringValue = "open_file_picker";

And depending on which parser you're using nowadays, the parser may give an "unescaped" string value.

Imagine a constant

public static String NEWLINE_UNIX = "\n";

now when a library like asm parses this, it will give you the "un-escaped" representation of string, i.e the actual bytes in this will be [ 10 ]. If you straightforwardly interpolate it in your generated code, you will get

const NEWLINE_UNIX = "
";

which is not valid dart code.

It's a good idea to be exhaustive when handling such cases. So you may have to implement the equivalent of Python repr, or golangs strconv.Quote to ensure all string literals parsed from Java can be safely and readably quoted in Dart. This function would probably be a switch case for ASCII character ranges (depending on dart escaping rules) plus some handling of unicode characters.

Alternatively, you may be able to sidestep this quoting-escaping dilema and generate something like this if Dart allows such constant computations (forgive me for forgetting Dart syntax)

const UNIX_NEWLINE$string_value = String.fromCodePointArray([10])

mahesh-hegde avatar Jan 02 '25 17:01 mahesh-hegde

ACTIIN_SOMETHING$string_value - looks a little ugly

Ugly indeed, but I personally wouldn't be mad. On the other side, maybe it could be other way around? this is: ACTION_SMTH$javaStringRef - especially since int values are already static, and not references, it would be consistent

I finally ended up copy-pasting whole sdk source code and find-replace'ing all public statics into static const. While doing that, i left the documentation comments as-is and found it very useful when i could just ctrl+q in my intelij to look up which ACTION_SMTH did what. I know that java docs and darts are different etc etc, but it would be very much quality-of-life to copy-paste them like that in the jnigen :heart:

TheLastGimbus avatar Jan 03 '25 12:01 TheLastGimbus

I don't see how a non-exhaustive switch statement is better than if statements other than some aesthetic preference so this is not a high priority.

if (action == jni.BluetoothAdapter.ACTION_STATE_CHANGED) {
  // ...
} else if (action == jni.BluetoothAdapter.ACTION_BOND_STATE_CHANGED) {
  // ...
} // else if ...

HosseinYousefi avatar Jan 10 '25 11:01 HosseinYousefi