From e886f65dca32f5bcca1e7af3d77f83df8ccfa2de Mon Sep 17 00:00:00 2001 From: Yavor Ivanov Date: Tue, 10 Jun 2025 12:16:05 +0300 Subject: [PATCH 01/11] docs: Recommendations for autofix migrations --- docs/Development.md | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/docs/Development.md b/docs/Development.md index 3db691eaa..5135a02df 100644 --- a/docs/Development.md +++ b/docs/Development.md @@ -21,3 +21,33 @@ The script updates multiple places where the corresponding SAPUI5 types are refe - This file contains additional information that is not available or accessible via the TypeScript definitions. It is an extract from the original `api.json` files of the SAPUI5 libraries. - [`resources/types/pseudo-modules`](../resources/types/pseudo-modules) - This folder contains additional module declarations for the detection of pseudo modules. + +## Autofix Checklist + +When developing autofix solutions, we realized it's not always a smooth or straightforward process. To mitigate risks such as missed cases or incompatible migrations, we've compiled a checklist of best practices. + +Autofix solutions generally fall into two categories: + +* **1:1 Replacements** +* **Complex Replacements** + +--- + +### 1:1 Replacements + +* [ ] Function arguments have **exactly the same** type, order, and count. +* [ ] Return type of the replacement matches **exactly** the original return type. +* [ ] If the return type is complex (e.g., object or enum): + + * [ ] **Enum**: Contains **exactly the same** values. + * [ ] **Object**: Has **identical** properties. + * [ ] **Object methods**: Return values of any method in the returned object must have **the same types** as in the original version. + +--- + +### Complex Replacements + +* [ ] If the return type differs, use the `isExpectedValueExpression()` utility and the `fixHints.exportCodeToBeUsed.isExpectedValue` flag. Migrate only when the result is **not used or assigned** further (e.g., setter calls like `sap.ui.getCore().getConfig().setCalendarType(...)`). +* [ ] In legacy code, argument type checks or assertions are common. If the new solution doesn't handle these internally, ensure you can **statically verify** the argument types. If not, **skip** the migration. +* [ ] When arguments are **shuffled, merged, or modified**, ensure any **comments** around them are **preserved**. +* [ ] Maintain **whitespaces and line breaks**. Some expressions span multiple lines or are auto-formatted with tabs/spaces. From 1cb3e1f6500d460f4c22a1056ce60911f3cc0e9f Mon Sep 17 00:00:00 2001 From: Yavor Ivanov Date: Tue, 10 Jun 2025 12:26:48 +0300 Subject: [PATCH 02/11] docs: Add examples --- docs/Development.md | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/docs/Development.md b/docs/Development.md index 5135a02df..7b1b2bffd 100644 --- a/docs/Development.md +++ b/docs/Development.md @@ -35,7 +35,11 @@ Autofix solutions generally fall into two categories: ### 1:1 Replacements -* [ ] Function arguments have **exactly the same** type, order, and count. +* [ ] Function arguments have **exactly the same** type, order, value and count. +```js + // Should not be replaced as "startText" is more than one char and the old API behaves differently + var padLeft = jQuery.sap.padLeft(startsWithLetter, startText, 8); +``` * [ ] Return type of the replacement matches **exactly** the original return type. * [ ] If the return type is complex (e.g., object or enum): @@ -48,6 +52,39 @@ Autofix solutions generally fall into two categories: ### Complex Replacements * [ ] If the return type differs, use the `isExpectedValueExpression()` utility and the `fixHints.exportCodeToBeUsed.isExpectedValue` flag. Migrate only when the result is **not used or assigned** further (e.g., setter calls like `sap.ui.getCore().getConfig().setCalendarType(...)`). +```js + // The old API always returns a logger instance. So in case the return value + // is accessed, the call should not be migrated: + jQuery.sap.log.debug().info(); + const debug = (msg, logger) => logger ? logger(msg) : jQuery.sap.log.debug(msg); + debug("msg 2", jQuery.sap.log.debug("msg 1")); + debug("msg 2", (jQuery.sap.log.debug("msg 1"))); + debug("msg 2", ((((jQuery.sap.log.debug("msg 1")))))); + var debugInfo = jQuery.sap.log.debug(); + var info = { + debug: jQuery.sap.log.debug() + }; + jQuery.sap.log.debug() ?? jQuery.sap.log.info(); + jQuery.sap.log.debug() ? "a" : "b"; + jQuery.sap.log.debug(), jQuery.sap.log.info(); +``` * [ ] In legacy code, argument type checks or assertions are common. If the new solution doesn't handle these internally, ensure you can **statically verify** the argument types. If not, **skip** the migration. +```js + + var padLeft = jQuery.sap.padLeft("a", "0", 4); // "a".padStart(4, "0"); + var padLeft2 = jQuery.sap.padLeft("a", "0000", 4); // Not migrated. Value differs in old and new + var padLeft3 = jQuery.sap.padLeft(startsWithLetter, "0", 4); // startsWithLetter might not be possible to be determined +``` * [ ] When arguments are **shuffled, merged, or modified**, ensure any **comments** around them are **preserved**. + * [ ] Maintain **whitespaces and line breaks**. Some expressions span multiple lines or are auto-formatted with tabs/spaces. +```js + // Line breaks between property access and call + const myLogger = jQuery. + sap. + log. + getLogger(); + + // Spaces between property access, call, and arguments + jQuery . sap . log . error ( "error" ) ; +``` From 6dd294ef5fd0b7dafd5888188b150ce5bc916264 Mon Sep 17 00:00:00 2001 From: Yavor Ivanov Date: Tue, 10 Jun 2025 14:00:47 +0300 Subject: [PATCH 03/11] docs: Fix example --- docs/Development.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/Development.md b/docs/Development.md index 7b1b2bffd..6ff90a12d 100644 --- a/docs/Development.md +++ b/docs/Development.md @@ -37,8 +37,10 @@ Autofix solutions generally fall into two categories: * [ ] Function arguments have **exactly the same** type, order, value and count. ```js - // Should not be replaced as "startText" is more than one char and the old API behaves differently - var padLeft = jQuery.sap.padLeft(startsWithLetter, startText, 8); + // Should not be replaced as the second argument is more than one char and the old API behaves differently + var padLeft = jQuery.sap.padLeft("a", "Hello", 8); + + var padLeft = jQuery.sap.padLeft("a", "0", 8); // Will be migrated ``` * [ ] Return type of the replacement matches **exactly** the original return type. * [ ] If the return type is complex (e.g., object or enum): From d1950cdb3c61c97b80cd452f2dc4005368675024 Mon Sep 17 00:00:00 2001 From: Yavor Ivanov Date: Thu, 12 Jun 2025 11:47:20 +0300 Subject: [PATCH 04/11] docs: Update docs/Development.md Co-authored-by: Merlin Beutlberger --- docs/Development.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Development.md b/docs/Development.md index 6ff90a12d..80f52b71c 100644 --- a/docs/Development.md +++ b/docs/Development.md @@ -37,7 +37,7 @@ Autofix solutions generally fall into two categories: * [ ] Function arguments have **exactly the same** type, order, value and count. ```js - // Should not be replaced as the second argument is more than one char and the old API behaves differently + // This case should not be migrated to "String#padStart" because the second argument is longer than one char, which will behave differently with the String API var padLeft = jQuery.sap.padLeft("a", "Hello", 8); var padLeft = jQuery.sap.padLeft("a", "0", 8); // Will be migrated From 0870b8e1ef7caffa5341105051660eebe84f7e47 Mon Sep 17 00:00:00 2001 From: Yavor Ivanov Date: Thu, 12 Jun 2025 11:47:37 +0300 Subject: [PATCH 05/11] docs: Update docs/Development.md Co-authored-by: Merlin Beutlberger --- docs/Development.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Development.md b/docs/Development.md index 80f52b71c..b6166a1b0 100644 --- a/docs/Development.md +++ b/docs/Development.md @@ -53,7 +53,7 @@ Autofix solutions generally fall into two categories: ### Complex Replacements -* [ ] If the return type differs, use the `isExpectedValueExpression()` utility and the `fixHints.exportCodeToBeUsed.isExpectedValue` flag. Migrate only when the result is **not used or assigned** further (e.g., setter calls like `sap.ui.getCore().getConfig().setCalendarType(...)`). +* [ ] If the return type differs, migrate only those cases where it is **not used or assigned** (e.g., setter calls like `sap.ui.getCore().getConfig().setCalendarType(...)`). Use the `isExpectedValueExpression()` utility or the `mustNotUseReturnValue` flag available on some of the standard fix classes. ```js // The old API always returns a logger instance. So in case the return value // is accessed, the call should not be migrated: From 8c12918afb07d52469d04d2c361e0f94a1e2bd25 Mon Sep 17 00:00:00 2001 From: Yavor Ivanov Date: Thu, 12 Jun 2025 11:47:46 +0300 Subject: [PATCH 06/11] docs: Update docs/Development.md Co-authored-by: Merlin Beutlberger --- docs/Development.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Development.md b/docs/Development.md index b6166a1b0..12499e83e 100644 --- a/docs/Development.md +++ b/docs/Development.md @@ -55,7 +55,7 @@ Autofix solutions generally fall into two categories: * [ ] If the return type differs, migrate only those cases where it is **not used or assigned** (e.g., setter calls like `sap.ui.getCore().getConfig().setCalendarType(...)`). Use the `isExpectedValueExpression()` utility or the `mustNotUseReturnValue` flag available on some of the standard fix classes. ```js - // The old API always returns a logger instance. So in case the return value + // The old API always returns a logger instance, the new one returns undefined. So in case the return value // is accessed, the call should not be migrated: jQuery.sap.log.debug().info(); const debug = (msg, logger) => logger ? logger(msg) : jQuery.sap.log.debug(msg); From cdcc4180e0479aaf2b6f5a9b357de68fc4277d0b Mon Sep 17 00:00:00 2001 From: Yavor Ivanov Date: Thu, 12 Jun 2025 11:47:54 +0300 Subject: [PATCH 07/11] docs: Update docs/Development.md Co-authored-by: Merlin Beutlberger --- docs/Development.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Development.md b/docs/Development.md index 12499e83e..07eff1363 100644 --- a/docs/Development.md +++ b/docs/Development.md @@ -56,7 +56,7 @@ Autofix solutions generally fall into two categories: * [ ] If the return type differs, migrate only those cases where it is **not used or assigned** (e.g., setter calls like `sap.ui.getCore().getConfig().setCalendarType(...)`). Use the `isExpectedValueExpression()` utility or the `mustNotUseReturnValue` flag available on some of the standard fix classes. ```js // The old API always returns a logger instance, the new one returns undefined. So in case the return value - // is accessed, the call should not be migrated: + // is accessed, the call should not be migrated (i.e. all the cases below): jQuery.sap.log.debug().info(); const debug = (msg, logger) => logger ? logger(msg) : jQuery.sap.log.debug(msg); debug("msg 2", jQuery.sap.log.debug("msg 1")); From 257bb37e678c6c6c05b9a50bb54adb69b636650f Mon Sep 17 00:00:00 2001 From: Yavor Ivanov Date: Thu, 12 Jun 2025 11:48:02 +0300 Subject: [PATCH 08/11] docs: Update docs/Development.md Co-authored-by: Merlin Beutlberger --- docs/Development.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Development.md b/docs/Development.md index 07eff1363..c6b737d30 100644 --- a/docs/Development.md +++ b/docs/Development.md @@ -70,7 +70,7 @@ Autofix solutions generally fall into two categories: jQuery.sap.log.debug() ? "a" : "b"; jQuery.sap.log.debug(), jQuery.sap.log.info(); ``` -* [ ] In legacy code, argument type checks or assertions are common. If the new solution doesn't handle these internally, ensure you can **statically verify** the argument types. If not, **skip** the migration. +* [ ] Check the legacy API for argument type checks or assertions. If the new solution doesn't handle these internally, ensure to **statically verify** the argument types using the TypeScript TypeChecker. If they don't match with what the new API expects, **skip** the migration or, if easily possible, attempt to convert the type at runtime. ```js var padLeft = jQuery.sap.padLeft("a", "0", 4); // "a".padStart(4, "0"); From 99e2a8c7e835f431647c279f6b07c6146e60b2d5 Mon Sep 17 00:00:00 2001 From: Yavor Ivanov Date: Thu, 12 Jun 2025 11:48:08 +0300 Subject: [PATCH 09/11] docs: Update docs/Development.md Co-authored-by: Merlin Beutlberger --- docs/Development.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Development.md b/docs/Development.md index c6b737d30..19344e1eb 100644 --- a/docs/Development.md +++ b/docs/Development.md @@ -73,7 +73,7 @@ Autofix solutions generally fall into two categories: * [ ] Check the legacy API for argument type checks or assertions. If the new solution doesn't handle these internally, ensure to **statically verify** the argument types using the TypeScript TypeChecker. If they don't match with what the new API expects, **skip** the migration or, if easily possible, attempt to convert the type at runtime. ```js - var padLeft = jQuery.sap.padLeft("a", "0", 4); // "a".padStart(4, "0"); + var padLeft = jQuery.sap.padLeft("a", "0", 4); // Becomes "a".padStart(4, "0"); var padLeft2 = jQuery.sap.padLeft("a", "0000", 4); // Not migrated. Value differs in old and new var padLeft3 = jQuery.sap.padLeft(startsWithLetter, "0", 4); // startsWithLetter might not be possible to be determined ``` From dd24dc8a4c3a30b51365f83015d59a8b89873975 Mon Sep 17 00:00:00 2001 From: Yavor Ivanov Date: Thu, 12 Jun 2025 11:48:20 +0300 Subject: [PATCH 10/11] docs: Update docs/Development.md Co-authored-by: Merlin Beutlberger --- docs/Development.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Development.md b/docs/Development.md index 19344e1eb..e232bfd7c 100644 --- a/docs/Development.md +++ b/docs/Development.md @@ -74,7 +74,7 @@ Autofix solutions generally fall into two categories: ```js var padLeft = jQuery.sap.padLeft("a", "0", 4); // Becomes "a".padStart(4, "0"); - var padLeft2 = jQuery.sap.padLeft("a", "0000", 4); // Not migrated. Value differs in old and new + var padLeft2 = jQuery.sap.padLeft("a", "0000", 4); // Must not be migrated. The second argument contains more than one character, which will yield different results with the new API var padLeft3 = jQuery.sap.padLeft(startsWithLetter, "0", 4); // startsWithLetter might not be possible to be determined ``` * [ ] When arguments are **shuffled, merged, or modified**, ensure any **comments** around them are **preserved**. From 349faaf4a8d0ffdf5cc140ba524a61d1066baa42 Mon Sep 17 00:00:00 2001 From: Yavor Ivanov Date: Thu, 12 Jun 2025 11:48:30 +0300 Subject: [PATCH 11/11] docs: Update docs/Development.md Co-authored-by: Merlin Beutlberger --- docs/Development.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Development.md b/docs/Development.md index e232bfd7c..31b8ff913 100644 --- a/docs/Development.md +++ b/docs/Development.md @@ -75,7 +75,7 @@ Autofix solutions generally fall into two categories: var padLeft = jQuery.sap.padLeft("a", "0", 4); // Becomes "a".padStart(4, "0"); var padLeft2 = jQuery.sap.padLeft("a", "0000", 4); // Must not be migrated. The second argument contains more than one character, which will yield different results with the new API - var padLeft3 = jQuery.sap.padLeft(startsWithLetter, "0", 4); // startsWithLetter might not be possible to be determined + var padLeft3 = jQuery.sap.padLeft(startsWithLetter, "0", 4); // It might not be possible to determine whether startsWithLetter is a string. In that case, do not migrate ``` * [ ] When arguments are **shuffled, merged, or modified**, ensure any **comments** around them are **preserved**.