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 <riitta-leena.miettinen@qt.io>
Reviewed-by: Tim Jenssen <tim.jenssen@qt.io>

src/plugins/cpptools/clangdiagnosticconfigswidget.cpp

@@ -28,6 +28,7 @@
 
 #include <utils/algorithm.h>
 #include <utils/qtcassert.h>
+#include <utils/utilsicons.h>
 
 #include <QDebug>
 #include <QInputDialog>
@@ -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
+    // Clean up input
-            = m_ui->diagnosticOptionsTextEdit->document()->toPlainText().trimmed();
+    const QString diagnosticOptions = m_ui->diagnosticOptionsTextEdit->document()->toPlainText();
-    const QStringList updatedCommandLine
+    const QStringList normalizedOptions = normalizeDiagnosticInputOptions(diagnosticOptions);
-            = diagnosticOptions.trimmed().split(QLatin1Char(' '), QString::SkipEmptyParts);
 
+    // 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(' '));
+    const QString options = m_notAcceptedOptions.contains(config.id())
-    setDiagnosticOptions(commandLineWarnings);
+            ? 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,

src/plugins/cpptools/clangdiagnosticconfigswidget.h

@@ -30,6 +30,7 @@
 #include "clangdiagnosticconfig.h"
 #include "clangdiagnosticconfigsmodel.h"
 
+#include <QHash>
 #include <QWidget>
 
 namespace CppTools {
@@ -72,6 +73,7 @@ private:
     const ClangDiagnosticConfig &currentConfig() 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<Core::Id, QString> m_notAcceptedOptions;
 };
 
 } // CppTools namespace

src/plugins/cpptools/clangdiagnosticconfigswidget.ui

@@ -6,7 +6,7 @@
    <rect>
     <x>0</x>
     <y>0</y>
-    <width>597</width>
+    <width>665</width>
     <height>300</height>
    </rect>
   </property>
@@ -14,18 +14,6 @@
    <string>Form</string>
   </property>
   <layout class="QVBoxLayout" name="verticalLayout">
-   <property name="leftMargin">
-    <number>0</number>
-   </property>
-   <property name="topMargin">
-    <number>0</number>
-   </property>
-   <property name="rightMargin">
-    <number>0</number>
-   </property>
-   <property name="bottomMargin">
-    <number>0</number>
-   </property>
    <item>
     <layout class="QHBoxLayout" name="horizontalLayout">
      <item>
@@ -61,6 +49,37 @@
      </item>
     </layout>
    </item>
+   <item>
+    <layout class="QHBoxLayout" name="horizontalLayout_2">
+     <item>
+      <widget class="QLabel" name="validationResultIcon">
+       <property name="text">
+        <string>ValidationIcon</string>
+       </property>
+      </widget>
+     </item>
+     <item>
+      <widget class="QLabel" name="validationResultLabel">
+       <property name="text">
+        <string>ValidationText</string>
+       </property>
+      </widget>
+     </item>
+     <item>
+      <spacer name="horizontalSpacer">
+       <property name="orientation">
+        <enum>Qt::Horizontal</enum>
+       </property>
+       <property name="sizeHint" stdset="0">
+        <size>
+         <width>40</width>
+         <height>20</height>
+        </size>
+       </property>
+      </spacer>
+     </item>
+    </layout>
+   </item>
    <item>
     <widget class="QPlainTextEdit" name="diagnosticOptionsTextEdit"/>
    </item>