Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[FEATURE] builder: Improve support for ES6+ syntax #774

Merged
merged 11 commits into from
Sep 19, 2022
Merged

[FEATURE] builder: Improve support for ES6+ syntax #774

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f3df91a
[INTERNAL] Improve support for ES6+ Syntax
matz3 Jul 29, 2022
8842249
[INTERNAL] Add tests with async factory function
flovogt Aug 23, 2022
62a8e5b
JSModuleAnalyzer: Conditional handling of LogicalExpression
matz3 Aug 23, 2022
d9d3d2a
[INTERNAL] log warning when asynchronous factory function is used for…
flovogt Aug 25, 2022
e4783b1
[INTERNAL] FioriElementsAnalyzer/SmartTemplateAnalyzer: Add tests for…
flovogt Aug 25, 2022
bd0dd80
[INTERNAL] move sinon sanbox creation into beforeEach hook and enable…
flovogt Aug 29, 2022
dd123ff
[INTERNAL] log module name when async factory function is used
flovogt Aug 29, 2022
78a3b0f
Revert async warning log
matz3 Sep 19, 2022
30bb341
Typo
matz3 Sep 19, 2022
2681467
Improve comment
matz3 Sep 19, 2022
d5ce05a
ASTUtils: Add missing test cases
matz3 Sep 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 27 additions & 12 deletions lib/lbt/analyzer/FioriElementsAnalyzer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
const ModuleName = require("../utils/ModuleName");
const SapUiDefine = require("../calls/SapUiDefine");
const {parseJS, Syntax} = require("../utils/parseUtils");
const {getValue, isMethodCall, isString} = require("../utils/ASTUtils");
const {getValue, isMethodCall, getStringValue} = require("../utils/ASTUtils");
const log = require("@ui5/logger").getLogger("lbt:analyzer:FioriElementAnalyzer");

// ---------------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -165,24 +165,39 @@ class FioriElementsAnalyzer {
const TA = defineCall.findImportName("sap/fe/core/TemplateAssembler.js");
// console.log("local name for TemplateAssembler: %s", TA);
if ( TA && defineCall.factory ) {
defineCall.factory.body.body.forEach( (stmt) => {
if ( stmt.type === Syntax.ReturnStatement &&
isMethodCall(stmt.argument, [TA, "getTemplateComponent"]) &&
stmt.argument.arguments.length > 2 &&
stmt.argument.arguments[2].type === "ObjectExpression" ) {
templateName = this._analyzeTemplateClassDefinition(stmt.argument.arguments[2]) || templateName;
if (defineCall.factory.type === Syntax.ArrowFunctionExpression &&
defineCall.factory.expression === true) {
if ( this._isTemplateClassDefinition(TA, defineCall.factory.body) ) {
templateName =
this._analyzeTemplateClassDefinition(defineCall.factory.body.arguments[2]) || templateName;
}
});
} else {
defineCall.factory.body.body.forEach( (stmt) => {
if ( stmt.type === Syntax.ReturnStatement &&
this._isTemplateClassDefinition(TA, stmt.argument)
) {
templateName =
this._analyzeTemplateClassDefinition(stmt.argument.arguments[2]) || templateName;
}
});
}
if (defineCall.factory.async) {
flovogt marked this conversation as resolved.
Show resolved Hide resolved
log.warn("Using 'sap.ui.define' with an asynchronous function callback " +
"is currently not supported by the UI5 runtime.");
}
}
}
return templateName;
}

_isTemplateClassDefinition(TA, node) {
return isMethodCall(node, [TA, "getTemplateComponent"]) &&
node.arguments.length > 2 &&
node.arguments[2].type === "ObjectExpression";
}

_analyzeTemplateClassDefinition(clazz) {
const defaultValue = getValue(clazz, ["metadata", "properties", "templateName", "defaultValue"]);
if ( isString(defaultValue) ) {
return defaultValue.value;
}
return getStringValue(getValue(clazz, ["metadata", "properties", "templateName", "defaultValue"]));
}
}

Expand Down
141 changes: 88 additions & 53 deletions lib/lbt/analyzer/JSModuleAnalyzer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ const escope = require("escope");
const ModuleName = require("../utils/ModuleName");
const {Format: ModuleFormat} = require("../resources/ModuleInfo");
const UI5ClientConstants = require("../UI5ClientConstants");
const {findOwnProperty, getLocation, getPropertyKey, isMethodCall, isString} = require("../utils/ASTUtils");
const {
findOwnProperty, getLocation, getPropertyKey,
isMethodCall, isString, getStringValue} = require("../utils/ASTUtils");
const log = require("@ui5/logger").getLogger("lbt:analyzer:JSModuleAnalyzer");

// ------------------------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -64,7 +66,7 @@ const EnrichedVisitorKeys = (function() {
BreakStatement: [],
CallExpression: [], // special handling
CatchClause: ["param", "body"],
ChainExpression: [],
ChainExpression: ["expression"],
ClassBody: [],
ClassDeclaration: [],
ClassExpression: [],
Expand Down Expand Up @@ -114,7 +116,7 @@ const EnrichedVisitorKeys = (function() {
ImportSpecifier: [], // imported, local
Literal: [],
LabeledStatement: [],
LogicalExpression: [],
LogicalExpression: ["right"],
MemberExpression: [],
MetaProperty: toBeDone(["meta", "property"]),
MethodDefinition: [],
Expand Down Expand Up @@ -541,21 +543,27 @@ class JSModuleAnalyzer {

function onDeclare(node) {
const args = node.arguments;
if ( args.length > 0 && isString(args[0]) ) {
const name = ModuleName.fromUI5LegacyName( args[0].value );
if ( nModuleDeclarations === 1 && !mainModuleFound) {
// if this is the first declaration, then this is the main module declaration
// note that this overrides an already given name
setMainModuleInfo(name, getDocumentation(node));
} else if ( nModuleDeclarations > 1 && name === info.name ) {
// ignore duplicate declarations (e.g. in behavior file of design time controls)
log.warn(`duplicate declaration of module name at ${getLocation(args)} in ${name}`);
if (args.length > 0) {
const value = getStringValue(args[0]);
if (value !== undefined) {
const name = ModuleName.fromUI5LegacyName(value);
if ( nModuleDeclarations === 1 && !mainModuleFound) {
// if this is the first declaration, then this is the main module declaration
// note that this overrides an already given name
setMainModuleInfo(name, getDocumentation(node));
} else if ( nModuleDeclarations > 1 && name === info.name ) {
// ignore duplicate declarations (e.g. in behavior file of design time controls)
log.warn(`duplicate declaration of module name at ${getLocation(args)} in ${name}`);
} else {
// otherwise it is just a submodule declaration
info.addSubModule(name);
}
return;
} else {
// otherwise it is just a submodule declaration
info.addSubModule(name);
log.error("jQuery.sap.declare: module name could not be determined from first argument:", args[0]);
}
} else {
log.error("jQuery.sap.declare: module name could not be determined from first argument:", args[0]);
log.error("jQuery.sap.declare: module name could not be determined, no arguments are given");
}
}

Expand All @@ -569,20 +577,26 @@ class JSModuleAnalyzer {

// determine the name of the module
let name = null;
if ( i < nArgs && isString(args[i]) ) {
name = ModuleName.fromRequireJSName( args[i++].value );
if ( name === defaultName ) {
// hardcoded name equals the file name, so this definition qualifies as main module definition
setMainModuleInfo(name, desc);
} else {
info.addSubModule(name);
if ( candidateName == null ) {
// remember the name and description in case no other module qualifies as main module
candidateName = name;
candidateDescription = desc;
if ( i < nArgs ) {
const value = getStringValue( args[i] );
if ( value !== undefined ) {
name = ModuleName.fromRequireJSName(value);
if ( name === defaultName ) {
// hardcoded name equals the file name, so this definition qualifies as main module definition
setMainModuleInfo(name, desc);
} else {
info.addSubModule(name);
if ( candidateName == null ) {
// remember the name and description in case no other module qualifies as main module
candidateName = name;
candidateDescription = desc;
}
}
i++;
}
} else {
}

if ( !name ) {
nUnnamedDefines++;
if ( nUnnamedDefines > 1 ) {
throw new Error(
Expand Down Expand Up @@ -614,15 +628,25 @@ class JSModuleAnalyzer {
// UI5 signature with one or many required modules
for (let i = 0; i < nArgs; i++) {
const arg = args[i];
if ( isString(arg) ) {
const requiredModuleName = ModuleName.fromUI5LegacyName( arg.value );
const value = getStringValue(arg);
if ( value !== undefined ) {
const requiredModuleName = ModuleName.fromUI5LegacyName( value );
info.addDependency(requiredModuleName, conditional);
} else if ( arg.type == Syntax.ConditionalExpression &&
isString(arg.consequent) && isString(arg.alternate) ) {
const requiredModuleName1 = ModuleName.fromUI5LegacyName( arg.consequent.value );
info.addDependency(requiredModuleName1, true);
const requiredModuleName2 = ModuleName.fromUI5LegacyName( arg.alternate.value );
info.addDependency(requiredModuleName2, true);
} else if ( arg.type == Syntax.ConditionalExpression) {
const consequentValue = getStringValue(arg.consequent);
const alternateValue = getStringValue(arg.alternate);
if ( consequentValue !== undefined ) {
const requiredModuleName1 = ModuleName.fromUI5LegacyName( consequentValue );
info.addDependency(requiredModuleName1, true);
}
if ( alternateValue !== undefined ) {
const requiredModuleName2 = ModuleName.fromUI5LegacyName( alternateValue );
info.addDependency(requiredModuleName2, true);
}
if ( consequentValue === undefined || alternateValue === undefined ) {
log.verbose("jQuery.sap.require: cannot evaluate dynamic arguments: ", arg && arg.type);
info.dynamicDependencies = true;
}
} else {
log.verbose("jQuery.sap.require: cannot evaluate dynamic arguments: ", arg && arg.type);
info.dynamicDependencies = true;
Expand All @@ -637,9 +661,10 @@ class JSModuleAnalyzer {
const i = 0;

if ( i < nArgs ) {
if ( isString(args[i]) ) {
const value = getStringValue(args[i]);
if ( value !== undefined ) {
// sap.ui.requireSync does not support relative dependencies
const moduleName = ModuleName.fromRequireJSName( args[i].value );
const moduleName = ModuleName.fromRequireJSName( value );
info.addDependency(moduleName, conditional);
} else {
log.verbose("sap.ui.requireSync: cannot evaluate dynamic arguments: ", args[i] && args[i].type);
Expand All @@ -654,18 +679,24 @@ class JSModuleAnalyzer {
let i = 0;

// determine the name of the module
if ( i < nArgs && isString(args[i]) ) {
const moduleName = ModuleName.fromRequireJSName( args[i++].value );
info.addSubModule(moduleName);

// add dependencies
// to correctly identify dependencies e.g. of a library-preload
const elementArg = args[i++];
if (elementArg && elementArg.type === Syntax.ArrayExpression) {
elementArg.elements.forEach((element) => {
const dependencyName = ModuleName.resolveRelativeRequireJSName(moduleName, element.value);
info.addDependency(dependencyName, conditional);
});
if ( i < nArgs ) {
const value = getStringValue(args[i++]);
if ( value !== undefined ) {
const moduleName = ModuleName.fromRequireJSName( value );
info.addSubModule(moduleName);

// add dependencies
// to correctly identify dependencies e.g. of a library-preload
const elementArg = args[i++];
if (elementArg && elementArg.type === Syntax.ArrayExpression) {
elementArg.elements.forEach((element) => {
const dependencyName = ModuleName.resolveRelativeRequireJSName(moduleName,
getStringValue(element));
info.addDependency(dependencyName, conditional);
});
}
} else {
log.warn("sap.ui.predefine call has a non supported type for module name (ignored)");
}
} else {
log.warn("sap.ui.predefine call is missing a module name (ignored)");
Expand All @@ -692,6 +723,9 @@ class JSModuleAnalyzer {
if ( modules && modules.type == Syntax.ObjectExpression ) {
modules.properties.forEach( function(property) {
let moduleName = getPropertyKey(property);
if ( !moduleName ) {
return;
}
if ( namesUseLegacyNotation ) {
moduleName = ModuleName.fromUI5LegacyName(moduleName);
}
Expand All @@ -705,16 +739,17 @@ class JSModuleAnalyzer {
function analyzeDependencyArray(array, conditional, name) {
// console.log(array);
array.forEach( (item) => {
if ( isString(item) ) {
const value = getStringValue(item);
if ( value !== undefined ) {
// ignore special AMD dependencies (require, exports, module)
if ( SPECIAL_AMD_DEPENDENCIES.indexOf(item.value) >= 0 ) {
if ( SPECIAL_AMD_DEPENDENCIES.indexOf(value) >= 0 ) {
return;
}
let requiredModule;
if (name == null) {
requiredModule = ModuleName.fromRequireJSName( item.value );
requiredModule = ModuleName.fromRequireJSName( value );
} else {
requiredModule = ModuleName.resolveRelativeRequireJSName(name, item.value);
requiredModule = ModuleName.resolveRelativeRequireJSName(name, value);
}
info.addDependency( requiredModule, conditional );
} else {
Expand Down
39 changes: 27 additions & 12 deletions lib/lbt/analyzer/SmartTemplateAnalyzer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
const ModuleName = require("../utils/ModuleName");
const SapUiDefine = require("../calls/SapUiDefine");
const {parseJS, Syntax} = require("../utils/parseUtils");
const {getValue, isMethodCall, isString} = require("../utils/ASTUtils");
const {getValue, isMethodCall, getStringValue} = require("../utils/ASTUtils");
const log = require("@ui5/logger").getLogger("lbt:analyzer:SmartTemplateAnalyzer");

// ---------------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -134,24 +134,39 @@ class TemplateComponentAnalyzer {
const TA = defineCall.findImportName("sap/suite/ui/generic/template/lib/TemplateAssembler.js");
// console.log("local name for TemplateAssembler: %s", TA);
if ( TA && defineCall.factory ) {
defineCall.factory.body.body.forEach( (stmt) => {
if ( stmt.type === Syntax.ReturnStatement &&
isMethodCall(stmt.argument, [TA, "getTemplateComponent"]) &&
stmt.argument.arguments.length > 2 &&
stmt.argument.arguments[2].type === "ObjectExpression" ) {
templateName = this._analyzeTemplateClassDefinition(stmt.argument.arguments[2]) || templateName;
if (defineCall.factory.type === Syntax.ArrowFunctionExpression &&
defineCall.factory.expression === true) {
if ( this._isTemplateClassDefinition(TA, defineCall.factory.body) ) {
templateName =
this._analyzeTemplateClassDefinition(defineCall.factory.body.arguments[2]) || templateName;
}
});
} else {
defineCall.factory.body.body.forEach( (stmt) => {
if ( stmt.type === Syntax.ReturnStatement &&
this._isTemplateClassDefinition(TA, stmt.argument)
) {
templateName =
this._analyzeTemplateClassDefinition(stmt.argument.arguments[2]) || templateName;
}
});
}
if (defineCall.factory.async) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This message is IMO misleading. The ui5loader can handle async callbacks. The returned promise is just used as export value and delivered to anyone importing the module.

Here, the smart templates (FE v2) cannot handle such a promise. That's the reason why we should log a message.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Frank. Correct. So what should we log in case of the XMLComposite here the artefact can be placed in the view declaratively. The sap.ui.core.mvc.XMLView does not support returning a promise, correct? But do we know who consumes the XMLComposite. A typed view might import the asynchronously defined XMLComposite and can handle the asynchronism.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's handle this in a follow-up PR, so that we can merge the current changes.

log.warn("Using 'sap.ui.define' with an asynchronous function callback " +
"is currently not supported by the UI5 runtime.");
}
}
}
return templateName;
}

_isTemplateClassDefinition(TA, node) {
return isMethodCall(node, [TA, "getTemplateComponent"]) &&
node.arguments.length > 2 &&
node.arguments[2].type === "ObjectExpression";
}

_analyzeTemplateClassDefinition(clazz) {
const defaultValue = getValue(clazz, ["metadata", "properties", "templateName", "defaultValue"]);
if ( isString(defaultValue) ) {
return defaultValue.value;
}
return getStringValue(getValue(clazz, ["metadata", "properties", "templateName", "defaultValue"]));
}
}

Expand Down
Loading