Bug 377617

Summary:	clazy-qstring-left makes false assumption
Product:	[Developer tools] clazy	Reporter:	Markus Enzenberger <markus.enzenberger>
Component:	general	Assignee:	Sergio Martins <smartins>
Status:	RESOLVED FIXED
Severity:	normal	CC:	markus.enzenberger, smartins
Priority:	NOR
Version First Reported In:	unspecified
Target Milestone:	---
Platform:	Other
OS:	Linux
Latest Commit:	https://commits.kde.org/clazy/dcc6a8500bd079fe2a0d62a19fe177bc892ac897	Version Fixed In:
Sentry Crash Report:

Description Markus Enzenberger 2017-03-14 15:16:47 UTC

The clazy-qstring-left warning makes the false assumption that QString::left(1) can be replaced by the more efficient QString::at(0). This is not true in general.

It is illegal to call QString::at() with an invalid position, but QString::left() may be called with an empty string (in which case it also returns an empty string).

There could exist code, which relies on this and uses QString::left(1) to avoid handling the extra case in the calling code.

Comment 1 Sergio Martins 2017-03-18 11:43:23 UTC

Git commit 4234fabc16cbdf3d14ab05fc20f251fcb04719b2 by Sergio Martins.
Committed on 18/03/2017 at 11:43.
Pushed by smartins into branch '1.1'.

qstring-left: Improve readme

at(0) asserts if string is empty, while left(1) does not

M  +7    -2    src/checks/level1/README-qstring-left.md

https://commits.kde.org/clazy/4234fabc16cbdf3d14ab05fc20f251fcb04719b2

Comment 2 Sergio Martins 2017-03-18 11:47:50 UTC

While that's true, it's still faster to call !isEmpty()+at(0) since they don't allocate memory.

I don't see any solution besides improving the readme

Comment 3 Markus Enzenberger 2017-03-18 17:28:34 UTC

The danger that I see right now is that someone runs clazy on code they didn't write, simply follows the warning to replace left(1) by at(0) and thereby introduces a new, maybe subtle and rarely triggered bug.

The optimal solution would be that clazy recognizes if a check for size 0 already exists in the code.

But as a short-time fix, maybe the warning message could be improved to mention that left(1) can be replaced by at(0) if the string is not empty?

Comment 4 Sergio Martins 2017-03-19 12:27:23 UTC

Git commit dcc6a8500bd079fe2a0d62a19fe177bc892ac897 by Sergio Martins.
Committed on 19/03/2017 at 12:26.
Pushed by smartins into branch '1.1'.

qstring-left: Warn the user to make sure string isn't empty

left(1) doesn't assert if string is empty, but at(0) does, so
inform the user about that in the warning message

M  +1    -1    src/checks/level1/qstring-left.cpp
M  +1    -1    tests/qstring-left/main.cpp.expected

https://commits.kde.org/clazy/dcc6a8500bd079fe2a0d62a19fe177bc892ac897