From 082ba1d13357321556ade456d572ba949c211e20 Mon Sep 17 00:00:00 2001 From: Nikolai Kosjar Date: Fri, 8 Sep 2017 13:00:44 +0200 Subject: [PATCH] Clang: Validate warnings options from user ...in "Menu: Tools > C++ > Tab: Code Model > Clang Code Model Warnings" to avoid (re)parse issues due an invalid command line for libclang. If an invalid option is detected, then indicate it by providing an error message. As long as there is an invalid option, the options are not accepted. We do not want to maintain a white list of valid options, so mostly check whether the option starts with "-W". An unknown or misspelled option like "-WnotYetKnown" will be ignored by libclang. To keep passing in some options for testing/debugging convenient, skip validation if QTC_CLANG_NO_DIAGNOSTIC_CHECK is set. Task-number: QTCREATORBUG-18864 Change-Id: I196d0474c7521969c54133f52dfc7be0394bfc3e Reviewed-by: Leena Miettinen Reviewed-by: Tim Jenssen --- .../cpptools/clangdiagnosticconfigswidget.cpp | 87 +++++++++++++++++-- .../cpptools/clangdiagnosticconfigswidget.h | 3 + .../cpptools/clangdiagnosticconfigswidget.ui | 45 +++++++--- 3 files changed, 115 insertions(+), 20 deletions(-) diff --git a/src/plugins/cpptools/clangdiagnosticconfigswidget.cpp b/src/plugins/cpptools/clangdiagnosticconfigswidget.cpp index 6b8a04ac337..288b5e3a8d7 100644 --- a/src/plugins/cpptools/clangdiagnosticconfigswidget.cpp +++ b/src/plugins/cpptools/clangdiagnosticconfigswidget.cpp @@ -28,6 +28,7 @@ #include #include +#include #include #include @@ -107,15 +108,61 @@ void ClangDiagnosticConfigsWidget::onRemoveButtonClicked() syncConfigChooserToModel(); } +static bool isAcceptedWarningOption(const QString &option) +{ + return option == "-w" + || option == "-pedantic" + || option == "-pedantic-errors"; +} + +// Reference: +// https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html +// https://clang.llvm.org/docs/DiagnosticsReference.html +static bool isValidOption(const QString &option) +{ + if (option == "-Werror") + return false; // Avoid errors due to unknown or misspelled warnings. + return option.startsWith("-W") || isAcceptedWarningOption(option); +} + +static QString validateDiagnosticOptions(const QStringList &options) +{ + // This is handy for testing, allow disabling validation. + if (qEnvironmentVariableIntValue("QTC_CLANG_NO_DIAGNOSTIC_CHECK")) + return QString(); + + for (const QString &option : options) { + if (!isValidOption(option)) + return ClangDiagnosticConfigsWidget::tr("Option \"%1\" is invalid.").arg(option); + } + + return QString(); +} + +static QStringList normalizeDiagnosticInputOptions(const QString &options) +{ + return options.simplified().split(QLatin1Char(' '), QString::SkipEmptyParts); +} + void ClangDiagnosticConfigsWidget::onDiagnosticOptionsEdited() { - const QString diagnosticOptions - = m_ui->diagnosticOptionsTextEdit->document()->toPlainText().trimmed(); - const QStringList updatedCommandLine - = diagnosticOptions.trimmed().split(QLatin1Char(' '), QString::SkipEmptyParts); + // Clean up input + const QString diagnosticOptions = m_ui->diagnosticOptionsTextEdit->document()->toPlainText(); + const QStringList normalizedOptions = normalizeDiagnosticInputOptions(diagnosticOptions); + // Validate + const QString errorMessage = validateDiagnosticOptions(normalizedOptions); + updateValidityWidgets(errorMessage); + if (!errorMessage.isEmpty()) { + // Remember the entered options in case the user will switch back. + m_notAcceptedOptions.insert(currentConfigId(), diagnosticOptions); + return; + } + m_notAcceptedOptions.remove(currentConfigId()); + + // Commit valid changes ClangDiagnosticConfig updatedConfig = currentConfig(); - updatedConfig.setCommandLineWarnings(updatedCommandLine); + updatedConfig.setCommandLineWarnings(normalizedOptions); m_diagnosticConfigsModel.appendOrUpdate(updatedConfig); emit customConfigsChanged(customConfigs()); @@ -167,8 +214,10 @@ void ClangDiagnosticConfigsWidget::syncOtherWidgetsToComboBox() m_ui->removeButton->setEnabled(!config.isReadOnly()); // Update child widgets - const QString commandLineWarnings = config.commandLineWarnings().join(QLatin1Char(' ')); - setDiagnosticOptions(commandLineWarnings); + const QString options = m_notAcceptedOptions.contains(config.id()) + ? m_notAcceptedOptions.value(config.id()) + : config.commandLineWarnings().join(QLatin1Char(' ')); + setDiagnosticOptions(options); m_ui->diagnosticOptionsTextEdit->setReadOnly(config.isReadOnly()); } @@ -186,11 +235,35 @@ void ClangDiagnosticConfigsWidget::setDiagnosticOptions(const QString &options) { if (options != m_ui->diagnosticOptionsTextEdit->document()->toPlainText()) { disconnectDiagnosticOptionsChanged(); + m_ui->diagnosticOptionsTextEdit->document()->setPlainText(options); + const QString errorMessage + = validateDiagnosticOptions(normalizeDiagnosticInputOptions(options)); + updateValidityWidgets(errorMessage); + connectDiagnosticOptionsChanged(); } } +void ClangDiagnosticConfigsWidget::updateValidityWidgets(const QString &errorMessage) +{ + QString validationResult; + const Utils::Icon *icon = nullptr; + QString styleSheet; + if (errorMessage.isEmpty()) { + icon = &Utils::Icons::INFO; + validationResult = tr("Configuration passes sanity checks."); + } else { + icon = &Utils::Icons::CRITICAL; + validationResult = tr("%1").arg(errorMessage); + styleSheet = "color: red;"; + } + + m_ui->validationResultIcon->setPixmap(icon->pixmap()); + m_ui->validationResultLabel->setText(validationResult); + m_ui->validationResultLabel->setStyleSheet(styleSheet); +} + void ClangDiagnosticConfigsWidget::connectConfigChooserCurrentIndex() { connect(m_ui->configChooserComboBox, diff --git a/src/plugins/cpptools/clangdiagnosticconfigswidget.h b/src/plugins/cpptools/clangdiagnosticconfigswidget.h index e22de1b97d0..ff11815a229 100644 --- a/src/plugins/cpptools/clangdiagnosticconfigswidget.h +++ b/src/plugins/cpptools/clangdiagnosticconfigswidget.h @@ -30,6 +30,7 @@ #include "clangdiagnosticconfig.h" #include "clangdiagnosticconfigsmodel.h" +#include #include namespace CppTools { @@ -72,6 +73,7 @@ private: const ClangDiagnosticConfig ¤tConfig() const; void setDiagnosticOptions(const QString &options); + void updateValidityWidgets(const QString &errorMessage); void connectConfigChooserCurrentIndex(); void disconnectConfigChooserCurrentIndex(); @@ -81,6 +83,7 @@ private: private: Ui::ClangDiagnosticConfigsWidget *m_ui; ClangDiagnosticConfigsModel m_diagnosticConfigsModel; + QHash m_notAcceptedOptions; }; } // CppTools namespace diff --git a/src/plugins/cpptools/clangdiagnosticconfigswidget.ui b/src/plugins/cpptools/clangdiagnosticconfigswidget.ui index 25d0c0b598d..e07ffb7b063 100644 --- a/src/plugins/cpptools/clangdiagnosticconfigswidget.ui +++ b/src/plugins/cpptools/clangdiagnosticconfigswidget.ui @@ -6,7 +6,7 @@ 0 0 - 597 + 665 300 @@ -14,18 +14,6 @@ Form - - 0 - - - 0 - - - 0 - - - 0 - @@ -61,6 +49,37 @@ + + + + + + ValidationIcon + + + + + + + ValidationText + + + + + + + Qt::Horizontal + + + + 40 + 20 + + + + + +