Skip to content

[FEATURE]修改kotlin扩展Any.contains函数名,以及其他类似的相关扩展函数 #1402

New issue

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

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

Already on GitHub? Sign in to your account

Closed
ousc opened this issue Apr 24, 2023 · 5 comments
Labels
enhancement New feature or request fixed
Milestone

Comments

@ousc
Copy link

ousc commented Apr 24, 2023

问题和需求描述

在一次debug过程中,我们团队发现一句必定正确的逻辑判断永远返回false,最终我们发现是fastjson2间接引起的问题.

1.起因

fastjson2的kotlin扩展包为kotlin提供了许多扩展方法,如parseArray()parseObject()to<T>()into<T>()eval<T>()等,去年我们使用kotlin + springboot的项目将fastjson包升级到了2.0,升级的原因之一就是新版提供了kotlin支持,同时,我们引入kotlin扩展包的初期体验到了极大的方便:

  • 新的api和kotlin代码风格融合,在dsl语法的使用中简化了代码,提高了可读性。

但是,这些扩展函数的设计中,有少数几个扩展函数存在很大的问题。

拿我最开始的情况,举个例子:

kotlin.text包内String的contains函数是一个非常常用的函数,用于判断字符串的包含关系
自从2.0.4以来,fastjson2提供了一个没有限制作用域和类型的扩展函数Any.contains

@Suppress(
    "HasPlatformType",
    "NOTHING_TO_INLINE"
)
inline fun Any.contains(
    path: String
) = JSONPath.of(path).contains(this)

这个扩展函数经常会在调用String.contains(other)的时候被悄悄自动引入进来,导致一些非常隐含的报错,而且一旦引入,整个文件所有的contains都完了
如下图所示,它的优先级甚至比kotlin.text默认的contains(charSequence)还高。
截屏2023-04-24 11 05 47

作为一个写kotlin各种平台(android/后台/multiplatform)代码,并且设计过kotlin框架的人,我认为这些函数存在以下问题:

  1. 这些扩展函数没有作用域,他们在项目的任何地方都可以调用,而不是在某些特定的类内
  2. 这些函数为了达到使用简单,和系统提供的很多常用的函数重名,如contains,to(kotlin使用to作为二维元组的连接中缀,相当于JS里面的“:”,不过好在一般会使用"foo" to "bar",很少会使用"foo".to("bar"))等,函数名和参数里也没有体现出和JSON有关的任何标识
  3. 这些扩展函数的使用对象为Any,如containsevalwriteTo等,很多类可能有自己的contains、eval、writeTo函数,在调用时很可能会调错

请描述你建议的实现方案

  1. 修改函数名
    像Any.toJSONString这种就不会引起歧义,可以考虑修改函数名
    需要做的就是将有关的函数设置为废弃,修改函数名后重新发包(可以使用@deprecated + ReplaceWith)
@Deprecated(
    "The 'function xxx' xxx will be removed soon, please use xxxxx",
    ReplaceWith("xxx.xxx(xxx)", "xxxx")
)
fun xxx

我认为contains这个函数修改的优先级很高,其他名字中没有体现JSON的、加在Any上的扩展函数是一种很不负责任的起名方式,毕竟扩展函数会对整个项目所有的对象函数调用进行污染。

  1. 另一种解决方法,增加RequiresOptIn
    增加一个@RequiresOptIn(level = RequiresOptIn.Level.ERROR, message = "xxx")注解,在调用此类函数时需要额外加入一个Opt,及早发现调用了错误的函数,防止这一类bug的发生。
    例:
@RequiresOptIn(level = RequiresOptIn.Level.ERROR, message = "This is a function applied by fastjson2, please notice!")
@Retention(AnnotationRetention.BINARY)
@Target(AnnotationTarget.CLASS, AnnotationTarget.FUNCTION)
annotation class FastJsonForKotlin
@Suppress(
    "HasPlatformType",
    "NOTHING_TO_INLINE"
)
@FastJsonForKotlin
inline fun Any.contains(
    path: String
) = JSONPath.of(path).contains(this)

实际调用时:
截屏2023-04-24 12 04 57

@ousc ousc added the enhancement New feature or request label Apr 24, 2023
@ousc ousc changed the title [FEATURE]修改kotlin扩展String.contains函数名,以及其他类似的相关扩展函数 [FEATURE]修改kotlin扩展Any.contains函数名,以及其他类似的相关扩展函数 Apr 24, 2023
@ousc
Copy link
Author

ousc commented Apr 24, 2023

第二种解决方法不太合适,毕竟同一个文件里也可能在使用了fastjson2的contains的同时使用kotlin.text.contains,建议修改函数名。

@kraity
Copy link
Collaborator

kraity commented Apr 24, 2023

您好, 感谢您指出不足之处.
出现原因是因为导入的包优先级高, 即导入外包扩展函数 , 会调用外包的扩展函数

在您们项目中可能是使用通配导入 import com.alibaba.fastjson2.*, 从而JSONPath.contains优先级高于String.contains
目前在不更新版本情况下解决方案是, 按需导入拓展函数如 import com.alibaba.fastjson2.toJSONString,

@ousc
Copy link
Author

ousc commented Apr 24, 2023

您好, 感谢您指出不足之处.
出现原因是因为导入的包优先级高, 即导入外包扩展函数 , 会调用外包的扩展函数

在您们项目中可能是使用通配导入 import com.alibaba.fastjson2.*, 从而JSONPath.contains优先级高于String.contains
目前在不更新版本情况下解决方案是, 按需导入拓展函数如 import com.alibaba.fastjson2.toJSONString,

问题并不出现在通配符导入上,而在fastjson2提供的扩展函数名太过通用,很多情况下会造成使用的差错。
toJSONString这类函数一看就知道来自于json解析工具,而contains则需要额外判断包名(实际上,即使我不引用import com.alibaba.fastjson2.*,只要我引用了fastjson2的任何类,Any.contains的扩展函数依然排在前列,见下图

在我需要的情况下,我可以通过import as解决冲突,但是大多数情况下,这种问题总是悄悄出现,因为你并不知道什么时候,你常用的函数就被替换成了另一个工具提供的扩展函数。
截屏2023-04-24 23 03 54

@ousc
Copy link
Author

ousc commented Apr 24, 2023

您好, 感谢您指出不足之处. 出现原因是因为导入的包优先级高, 即导入外包扩展函数 , 会调用外包的扩展函数

在您们项目中可能是使用通配导入 import com.alibaba.fastjson2.*, 从而JSONPath.contains优先级高于String.contains 目前在不更新版本情况下解决方案是, 按需导入拓展函数如 import com.alibaba.fastjson2.toJSONString,

感谢您的回复,祝项目越来越好

wenshao pushed a commit that referenced this issue Apr 24, 2023
* refactor: remove conflicting functions (incompatible) #1402

* docs: update the comments of JSON.kt

* refactor: remove the tests of JSONPathTest.kt
@wenshao wenshao added this to the 2.0.30 milestone Apr 24, 2023
@wenshao wenshao added the fixed label Apr 29, 2023
@wenshao
Copy link
Member

wenshao commented May 6, 2023

@wenshao wenshao closed this as completed May 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fixed
Projects
None yet
Development

No branches or pull requests

3 participants