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

[RFE] An option to only show lines for removal #711

Open
Hi-Angel opened this issue Sep 6, 2019 · 4 comments
Open

[RFE] An option to only show lines for removal #711

Hi-Angel opened this issue Sep 6, 2019 · 4 comments

Comments

@Hi-Angel
Copy link

Hi-Angel commented Sep 6, 2019

Problem

While trying to use the tool to get rid of unused includes in Konsole sources, I found it very tedious to use. These 6 commits took, I think, about 2 hours of my time.

There are 2 reasons it took so much time: α) false-positives, β) it suggests to remove the same headers as it suggested to add. A typical IWYU output for konsole project:

/home/constantine/Projects/konsole/src/ColorSchemeEditor.cpp should add these lines:
#include <klocalizedstring.h>      // for i18nc
#include <kmessagewidget.h>        // for KMessageWidget, KMessageWidget::Wa...
#include <kwindowsystem.h>         // for KWindowSystem
#include <qabstractbutton.h>       // for QAbstractButton
#include <qabstractslider.h>       // for QAbstractSlider
#include <qboxlayout.h>            // for QVBoxLayout
#include <qbrush.h>                // for QBrush
#include <qbytearray.h>            // for QByteArray
#include <qcheckbox.h>             // for QCheckBox
#include <qcolor.h>                // for QColor
#include <qcolordialog.h>          // for QColorDialog
#include <qcompleter.h>            // for QCompleter
#include <qdialogbuttonbox.h>      // for QDialogButtonBox, operator|, QDial...
#include <qdir.h>                  // for QDir, QDir::AllEntries
#include <qfiledialog.h>           // for QFileDialog
#include <qfileinfo.h>             // for QFileInfo
#include <qfilesystemmodel.h>      // for QFileSystemModel
#include <qfontmetrics.h>          // for QFontMetrics
#include <qheaderview.h>           // for QHeaderView, QHeaderView::Stretch
#include <qicon.h>                 // for QIcon
#include <qimagereader.h>          // for QImageReader
#include <qlabel.h>                // for QLabel
#include <qlineedit.h>             // for QLineEdit
#include <qlist.h>                 // for QList
#include <qnamespace.h>            // for ItemIsEditable, ItemIsSelectable
#include <qpushbutton.h>           // for QPushButton
#include <qrect.h>                 // for QRect
#include <qslider.h>               // for QSlider
#include <qstringbuilder.h>        // for operator+, QStringBuilder
#include <qstringlist.h>           // for QStringList
#include <qstringliteral.h>        // for QStaticStringData, QStringLiteral
#include <qtablewidget.h>          // for QTableWidgetItem, QTableWidget
#include <qtoolbutton.h>           // for QToolButton
#include <qwidget.h>               // for QWidget
#include "Profile.h"               // for Konsole

/home/constantine/Projects/konsole/src/ColorSchemeEditor.cpp should remove these lines:
- #include <KLocalizedString>  // lines 35-35
- #include <KWindowSystem>  // lines 34-34
- #include <QColorDialog>  // lines 33-33
- #include <QCompleter>  // lines 26-26
- #include <QDialogButtonBox>  // lines 36-36
- #include <QFileDialog>  // lines 29-29
- #include <QFileInfo>  // lines 25-25
- #include <QFileSystemModel>  // lines 27-27
- #include <QFontMetrics>  // lines 24-24
- #include <QIcon>  // lines 28-28
- #include <QImageReader>  // lines 30-30
- #include <QPushButton>  // lines 37-37
- #include <QVBoxLayout>  // lines 38-38

The full include-list for /home/constantine/Projects/konsole/src/ColorSchemeEditor.cpp:
#include "ColorSchemeEditor.h"
#include <klocalizedstring.h>      // for i18nc
#include <kmessagewidget.h>        // for KMessageWidget, KMessageWidget::Wa...
#include <kwindowsystem.h>         // for KWindowSystem
#include <qabstractbutton.h>       // for QAbstractButton
#include <qabstractslider.h>       // for QAbstractSlider
#include <qboxlayout.h>            // for QVBoxLayout
#include <qbrush.h>                // for QBrush
#include <qbytearray.h>            // for QByteArray
#include <qcheckbox.h>             // for QCheckBox
#include <qcolor.h>                // for QColor
#include <qcolordialog.h>          // for QColorDialog
#include <qcompleter.h>            // for QCompleter
#include <qdialogbuttonbox.h>      // for QDialogButtonBox, operator|, QDial...
#include <qdir.h>                  // for QDir, QDir::AllEntries
#include <qfiledialog.h>           // for QFileDialog
#include <qfileinfo.h>             // for QFileInfo
#include <qfilesystemmodel.h>      // for QFileSystemModel
#include <qfontmetrics.h>          // for QFontMetrics
#include <qheaderview.h>           // for QHeaderView, QHeaderView::Stretch
#include <qicon.h>                 // for QIcon
#include <qimagereader.h>          // for QImageReader
#include <qlabel.h>                // for QLabel
#include <qlineedit.h>             // for QLineEdit
#include <qlist.h>                 // for QList
#include <qnamespace.h>            // for ItemIsEditable, ItemIsSelectable
#include <qpushbutton.h>           // for QPushButton
#include <qrect.h>                 // for QRect
#include <qslider.h>               // for QSlider
#include <qstringbuilder.h>        // for operator+, QStringBuilder
#include <qstringlist.h>           // for QStringList
#include <qstringliteral.h>        // for QStaticStringData, QStringLiteral
#include <qtablewidget.h>          // for QTableWidgetItem, QTableWidget
#include <qtoolbutton.h>           // for QToolButton
#include <qwidget.h>               // for QWidget
#include "CharacterColor.h"        // for ColorEntry, TABLE_COLORS
#include "ColorScheme.h"           // for ColorScheme, ColorSchemeWallpaper
#include "Profile.h"               // for Konsole
#include "ui_ColorSchemeEditor.h"  // for ColorSchemeEditor

Quiz question: how many includes does it suggest to remove?

Answer: none. Every header it suggested to remove was actually moved to the should add these lines part.

As results, using IWYU is pretty tedious. I though I might be using it wrong, so I googled up, and found this blogpost from 2013, last 2 paragraphs of which suggests it was always like this to use.

Solution

An option to only show lines that can be removed without adding any would greatly help.

@kimgr
Copy link
Contributor

kimgr commented Sep 15, 2019

For your β -- the added headers are not the same as the removed headers. IWYU blindly suggests adding the header that defines a symbol, it doesn't know that Qt has a facade header convention where Qxyz is preferred over qxyz.h. Since this is mostly too stupid, we use mappings to describe the relationship between private and public headers for third-party libraries. IWYU ships with three Qt mapping files for different versions, and more recently a script to re-generate Qt mappings from your local installation.

But I think the remove-only mode sounds interesting. Could you take a stab at a proof-of-concept patch?

@Hi-Angel
Copy link
Author

Thank you!

But I think the remove-only mode sounds interesting. Could you take a stab at a proof-of-concept patch?

This sounds interesting, but I don't think I will do this anytime soon. But thank you for the interest, I'll bear it in mind, so depending on circumstances, I may take a stab at it.

@GilesBathgate
Copy link

@kimgr I am also interested in using iwyu as a unnecessary includes linter. 'don't-include-what-you-don't-use' I tried removing the parsing of header additions in fix_includes.py ParseOneRecord but this didn't seem to work as expected, I suspect that as @Hi-Angel mentions the problem might be that IWYU is suggesting to add headers that also should be removed. Do you think instead the patch needs to be to the main iwyu source?

@kimgr
Copy link
Contributor

kimgr commented Feb 19, 2022

@GilesBathgate Yes, most of the analysis is done by include-what-you-use proper. The fix_includes script only tries to do safe edits based on its output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants