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.

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

UI5 Linter doesn't mark getSecurityToken as deprectaed #516

Open
pubmikeb opened this issue Feb 5, 2025 · 9 comments
Open

UI5 Linter doesn't mark getSecurityToken as deprectaed #516

pubmikeb opened this issue Feb 5, 2025 · 9 comments
Labels
enhancement New feature or request

Comments

@pubmikeb
Copy link

pubmikeb commented Feb 5, 2025

Expected Behavior

getSecurityToken() of sap.ui.model.odata.v2.ODataModel should be marked as deprecated.

Current Behavior

UI5 Linter doesn't mark getSecurityToken() as deprecated.

Steps to Reproduce the Issue

  1. Use in your code bade getSecurityToken()
  2. Run UI5 Linter: ui5lint

Context

  • UI5 linter version: 1.8.0
  • Node.js version: 20.18.1
  • npm version: 10.8.2
  • OS/Platform: SAP BAS
  • Other information regarding your environment (optional): SAP BAS
@pubmikeb pubmikeb added the bug Something isn't working label Feb 5, 2025
@codeworrior
Copy link
Member

Can you please share at least a few lines of code around the getSecurityToken call? I doubt that the call by itself is the issue. The linter's capability to detect a certain method call as deprecated highly depends on TypeScript's capability to infer the type of the callee.
Most likely, this is caused by the context, therefore I'm asking...

@pubmikeb
Copy link
Author

pubmikeb commented Feb 5, 2025

@codeworrior, thanks for your extremely prompt response!

The way I call getSecurityToken() is pretty standard:

sap.ui.define([
	"./BaseController",
	"sap/ui/model/odata/v2/ODataModel"
], function (BaseController, ODataModel) {
	"use strict";

	return BaseController.extend("org.company.app.controller.Main", {
		onInit: function() {
			const securityToken = ODataModel.getSecurityToken();
		}
	});
});

What's interesting, when I add a call of another deprecated module, e.g. sap/ui/core/util/ExportTypeCSV, UI5 Linter perfectly identifies usage of deprecated component/API.

@codeworrior
Copy link
Member

codeworrior commented Feb 5, 2025

Ehm, if I'm not mistaken, getSecurityToken is an instance method. Did you mean (new ODataModel()).getSecurityToken().

@pubmikeb
Copy link
Author

pubmikeb commented Feb 5, 2025

You're right, ODataModel.getSecurityToken() is the wrong way to call getSecurityToken(), when I replaced it with (new ODataModel()).getSecurityToken(), UI5 Linter was capable of identifying the problem.

However, in the original codebase, we have something like this.getOwnerComponent().getModel().getSecurityToken() and this call isn't detected as well. I'll check what exactly do I get when calling this.getOwnerComponent().getModel().

BTW, I assumed, that UI5 Linter just parses usage of a module's methods and checks it against the list of deprecated APIs.

@codeworrior
Copy link
Member

BTW, I assumed, that UI5 Linter just parses usage of a module's methods and checks it against the list of deprecated APIs.

Understandable, but we have methods like v4.ODataModel#getProperty which are deprecated, but other getProperty methods are not (and many similar cases with other common method names). That's why the linter relies on type inference.

Methods with a quite generic return type like getModel() or byId() obviously cause issues with this approach. For those method we plan to enhance the inferred type information with addtl. info derived from the manifest.json (types of models) and XML views (types of controls per ID). But that's not implemented yet.

@pubmikeb
Copy link
Author

pubmikeb commented Feb 5, 2025

Methods with a quite generic return type like getModel() or byId() obviously cause issues with this approach.

Perhaps, the proper JSDoc signatures can serve as a workaround, until the described functionality is implemented or until the codebase is migrated to TypeScript,

@codeworrior
Copy link
Member

Sorry, I have to correct myself. @matz3 reminded me that for Controller#byId, we already merge knowledge from the XMLViews into the signature of the controller's byId call. Just for getModel it's still missing.

@pubmikeb
Copy link
Author

pubmikeb commented Feb 5, 2025

Just for getModel it's still missing.

Great, then less job to be done!

@flovogt
Copy link
Member

flovogt commented Feb 6, 2025

Thanks a lot @pubmikeb for reporting this missing feature. We will implement this in backlog item CPOUI5FOUNDATION-1014 (SAP-internal).

@RandomByte RandomByte added enhancement New feature or request and removed bug Something isn't working labels Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants